New issue
Advanced search Search tips

Issue 701075 link

Starred by 13 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

SVG transform is not working

Reported by kunzha...@gmail.com, Mar 13 2017

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.98 Safari/537.36

Steps to reproduce the problem:
Create a SVG node with transform="matrix(...)", the node is not placed in correct location.

What is the expected behavior?

What went wrong?
Please attached screenshot, in 56.0.29.24.87 all blocks are in correct location. After upgrading to 57.0.2987.98, all blocks are stacked in 0,0.

Did this work before? N/A 

Chrome version: 57.0.2987.98  Channel: stable
OS Version: OS X 10.12.3
Flash Version:
 
56.0.2924.87.png
69.8 KB View Download
57.0.2987.98.png
39.8 KB View Download

Comment 1 by kunzha...@gmail.com, Mar 13 2017

This issue occurs both on Mac and Windows 

Comment 2 by kunzha...@gmail.com, Mar 13 2017

Here is how do I set the transform: (Code in Dart)

var tr = el.transform.baseVal.createSvgTransformFromMatrix(matrix);
if (el.transform.baseVal.numberOfItems == 0) {
    el.transform.baseVal.appendItem(tr);
} else {
    el.transform.baseVal.replaceItem(tr, 0);
}

Canary shows the same issue
Version 59.0.3040.0 (Official Build) canary (64-bit)

Please advise how can I get around this.
 Issue 701074  has been merged into this issue.
Components: -Blink Blink>SVG
Labels: OS-Windows

Comment 6 by f...@opera.com, Mar 13 2017

Labels: Needs-Feedback
I'm not able to reproduce the issue. Any chance you could provide a minimal testcase? (Preferably using JS, not Dart =))

(BTW, createSvgTransformFromMatrix is called createSVGTransformFromMatrix in the ECMAScript bindnings, I don't know what kind of mapping might be involved here, but hopefully that's not an issue.)

Comment 7 by kunzha...@gmail.com, Mar 13 2017

I'll try to reproduce it in a minimal testcase. Thanks.
Project Member

Comment 8 by sheriffbot@chromium.org, Mar 13 2017

Cc: f...@opera.com
Labels: -Needs-Feedback
Thank you for providing more feedback. Adding requester "fs@opera.com" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 9 by kunzha...@gmail.com, Mar 14 2017

Here is the test case. Please
1. unzip the attached file
2. run npm install.
3. run gulp to make build
4. load the web in dist folder in browser.

in the main.js, in line 28, 33, or 38, no matter where I set the transform in transform2 function (which use el.transform.baseVal), the red rectangle will not move to 300,300 position. (This works in 56 build).

If comment out transform2 and uncomment line 27 to set the transform through setAttribute then it works.
svg_transform_js.zip
71.6 KB Download
Labels: Needs-Triage-M57

Comment 11 by f...@opera.com, Mar 14 2017

Cc: -f...@opera.com
Labels: -OS-Windows -Type-Bug -OS-Mac Type-Bug-Regression
Status: Available (was: Unconfirmed)
Thank you, reproduced. Attached minimized TC.
issue701075.html
641 bytes View Download

Comment 12 by f...@opera.com, Mar 14 2017

Owner: f...@opera.com
Status: Assigned (was: Available)
Project Member

Comment 13 by bugdroid1@chromium.org, Mar 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8554eac989977ff9d7f82444c01a500430097919

commit 8554eac989977ff9d7f82444c01a500430097919
Author: fs <fs@opera.com>
Date: Tue Mar 14 14:11:36 2017

Invalidate SVG 'transform' pres. attribute style even if not attached

When the 'transform' attribute was manipulated via its SVG DOM
representation (SVGTransformList), the
presentation-attribute-style-is-dirty flag would not be set unless the
element had been attached.
Reorder the contents of the 'transform' branch in
SVGGraphicsElement::svgAttributeChanged so that the presentation
attribute style is always dirtied regardless of attachment status.

BUG= 701075 

Review-Url: https://codereview.chromium.org/2745053005
Cr-Commit-Position: refs/heads/master@{#456696}

[add] https://crrev.com/8554eac989977ff9d7f82444c01a500430097919/third_party/WebKit/LayoutTests/svg/transforms/svgdom-manipulation-before-attach-expected.html
[add] https://crrev.com/8554eac989977ff9d7f82444c01a500430097919/third_party/WebKit/LayoutTests/svg/transforms/svgdom-manipulation-before-attach.html
[modify] https://crrev.com/8554eac989977ff9d7f82444c01a500430097919/third_party/WebKit/Source/core/svg/SVGGraphicsElement.cpp

Project Member

Comment 14 by bugdroid1@chromium.org, Mar 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b4ea1268734a32066e965107a5bb0d3ac7cba5a1

commit b4ea1268734a32066e965107a5bb0d3ac7cba5a1
Author: fs <fs@opera.com>
Date: Tue Mar 14 14:56:50 2017

Dirty pres. attribute style on <svg> dimension change when not attached

When an <svg> wasn't attached, and it had its 'width'/'height' mutated
via the SVG DOM interfaces (SVG*Length), presentation attribute style
would not be dirtied. This could lead to an incorrect size being
computed in some cases.
Ensure that presentation attribute style is always updated if the <svg>
element is not attached.

BUG= 701075 

Review-Url: https://codereview.chromium.org/2747153002
Cr-Commit-Position: refs/heads/master@{#456702}

[add] https://crrev.com/b4ea1268734a32066e965107a5bb0d3ac7cba5a1/third_party/WebKit/LayoutTests/svg/custom/svgdom-manipulation-of-svg-root-width-before-attach-expected.html
[add] https://crrev.com/b4ea1268734a32066e965107a5bb0d3ac7cba5a1/third_party/WebKit/LayoutTests/svg/custom/svgdom-manipulation-of-svg-root-width-before-attach.html
[modify] https://crrev.com/b4ea1268734a32066e965107a5bb0d3ac7cba5a1/third_party/WebKit/Source/core/svg/SVGSVGElement.cpp

Comment 15 by f...@opera.com, Mar 14 2017

Status: Fixed (was: Assigned)

Comment 16 by f...@opera.com, Mar 15 2017

Labels: Merge-Request-58
Project Member

Comment 17 by sheriffbot@chromium.org, Mar 15 2017

Labels: -Merge-Request-58 Hotlist-Merge-Approved Merge-Approved-58
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 18 by bugdroid1@chromium.org, Mar 15 2017

Labels: -merge-approved-58 merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0a1ad993c60db5a0fe6e621e84c8a551481288d4

commit 0a1ad993c60db5a0fe6e621e84c8a551481288d4
Author: Fredrik Söderquist <fs@opera.com>
Date: Wed Mar 15 15:19:12 2017

Invalidate SVG 'transform' pres. attribute style even if not attached

When the 'transform' attribute was manipulated via its SVG DOM
representation (SVGTransformList), the
presentation-attribute-style-is-dirty flag would not be set unless the
element had been attached.
Reorder the contents of the 'transform' branch in
SVGGraphicsElement::svgAttributeChanged so that the presentation
attribute style is always dirtied regardless of attachment status.

BUG= 701075 

Review-Url: https://codereview.chromium.org/2745053005
Cr-Commit-Position: refs/heads/master@{#456696}
(cherry picked from commit 8554eac989977ff9d7f82444c01a500430097919)

Review-Url: https://codereview.chromium.org/2747783006 .
Cr-Commit-Position: refs/branch-heads/3029@{#203}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[add] https://crrev.com/0a1ad993c60db5a0fe6e621e84c8a551481288d4/third_party/WebKit/LayoutTests/svg/transforms/svgdom-manipulation-before-attach-expected.html
[add] https://crrev.com/0a1ad993c60db5a0fe6e621e84c8a551481288d4/third_party/WebKit/LayoutTests/svg/transforms/svgdom-manipulation-before-attach.html
[modify] https://crrev.com/0a1ad993c60db5a0fe6e621e84c8a551481288d4/third_party/WebKit/Source/core/svg/SVGGraphicsElement.cpp

Comment 19 by f...@opera.com, Mar 15 2017

Cc: schenney@chromium.org
Labels: -Pri-2 RegressionReachedStable RegressionFound-57 Regressed-57 Pri-1
(@schenney, adding tracking labels that seemed appropriate, fix as needed.)

Comment 20 by f...@opera.com, Mar 15 2017

 Issue 701842  has been merged into this issue.
This bug is affecting many of our customers and also breaks our online demos:
http://live.yworks.com/yfiles-for-html/2.0/view/graphviewer/index.html (step through the visualizations in the combobox and visualizations will appear at the wrong location, sometimes invisible due to clipping).
Is there any chance this regression can be fixed in the current version (57)? It's still a long way to 58 with 57 just being released.

Thanks.

Comment 22 by f...@opera.com, Mar 16 2017

The bar for getting stuff into stable is pretty high, and I'm sure if this would pass over. That being said, I guess we can try, because the fix should be local enough (and fairly low risk.)

I should probably also take this opportunity to ask that you test your application with one (or both) of the beta and dev channel, that way we can catch these slip-ups earlier and prevent them from reaching stable at all...
Thanks! - we *do* test our apps on Canary and Beta (half our team is using these versions in their day to day work) - however this kind of bug could only be detected manually (none of our automatic tests discovered it - just like none of the Chromium tests did) and it only happens in relatively rare situations and even then it only "fails slightly, not totally breaking the application", so this time it just slipped through...

Comment 24 by f...@opera.com, Mar 16 2017

Ok, that's good to hear. (One of the up-sides is that at least now we have a regression test for this case...)
Labels: Merge-Request-57
I would really like to get this into the M-57 stable release. We received 2 bug reports for the issue and it is affecting a significant number of users.

The fix is extremely local and very unlikely to cause other problems.
Project Member

Comment 26 by sheriffbot@chromium.org, Mar 16 2017

Labels: -Merge-Request-57 Hotlist-Merge-Review Merge-Review-57
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Labels: -Merge-Review-57 Merge-Approved-57
Merge is approved for M57 branch 2987.  That said, we already had release candidates in the process of being pushed before the release team saw these bugs, and we are not going to hold pushing them (since we have a number of other urgent regressions in the field we need to address).  If we respin M57 we'll have these fixes included.
Project Member

Comment 29 by bugdroid1@chromium.org, Mar 17 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1dd21c9d72d0ed9bc4c31f94f6a7b308e88bb20d

commit 1dd21c9d72d0ed9bc4c31f94f6a7b308e88bb20d
Author: Fredrik Söderquist <fs@opera.com>
Date: Fri Mar 17 08:36:57 2017

Invalidate SVG 'transform' pres. attribute style even if not attached

When the 'transform' attribute was manipulated via its SVG DOM
representation (SVGTransformList), the
presentation-attribute-style-is-dirty flag would not be set unless the
element had been attached.
Reorder the contents of the 'transform' branch in
SVGGraphicsElement::svgAttributeChanged so that the presentation
attribute style is always dirtied regardless of attachment status.

BUG= 701075 

Review-Url: https://codereview.chromium.org/2745053005
Cr-Commit-Position: refs/heads/master@{#456696}
(cherry picked from commit 8554eac989977ff9d7f82444c01a500430097919)

Review-Url: https://codereview.chromium.org/2755813004 .
Cr-Commit-Position: refs/branch-heads/2987@{#840}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[add] https://crrev.com/1dd21c9d72d0ed9bc4c31f94f6a7b308e88bb20d/third_party/WebKit/LayoutTests/svg/transforms/svgdom-manipulation-before-attach-expected.html
[add] https://crrev.com/1dd21c9d72d0ed9bc4c31f94f6a7b308e88bb20d/third_party/WebKit/LayoutTests/svg/transforms/svgdom-manipulation-before-attach.html
[modify] https://crrev.com/1dd21c9d72d0ed9bc4c31f94f6a7b308e88bb20d/third_party/WebKit/Source/core/svg/SVGGraphicsElement.cpp

Cc: kkaluri@chromium.org
Labels: TE-Verified-M59 TE-Verified-59.0.3043.0
Verified this issue on Windows 10, Ubuntu 14.04 and Mac 10.12.3 with chrome #59.0.3043.0 using TestCase provided in comment #11.Didn't observe red color in the TestCase output.

Attaching the screen-cast for reference.

Hence adding TE-Verified labels.
Issue 701075.mp4
214 KB View Download
Android Verification: Verfied the issue using TestCase provided in comment #11 and works as per expected behavior, green color rectangle is shown in the output.

Tested on Latest M57,M58 Beta and M59 Dev.
Labels: TE-Verified-M58 TE-Verified-58.0.3029.33
Verified this issue on Windows 10, Ubuntu 14.04 and Mac 10.12.3 with chrome #58.0.3029.33 using TestCase provided in comment #11. Didn't observe red color in the TestCase output.

Attaching the screen-cast for reference.

Hence adding TE-Verified labels.
Issue 701075-M58.mp4
406 KB View Download
Labels: TE-Verified-57.0.2987.123 TE-Verified-M57
Verified the issue on Mac 10.12.3, Ubuntu 14.04 and Win 10 using stable 57.0.2987.123 using steps from Comment #11 and its working fine. 

Sign in to add a comment