Last Comment Bug 722882 - Regression in scripted SVG attribute changes
: Regression in scripted SVG attribute changes
Status: VERIFIED FIXED
[qa!]
: regression
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla13
Assigned To: Robert Longson
:
:
Mentors:
Depends on:
Blocks: 689546
  Show dependency treegraph
 
Reported: 2012-01-31 14:43 PST by Boris Zbarsky [:bz] (still a bit busy)
Modified: 2012-02-10 12:51 PST (History)
7 users (show)
longsonr: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
-
verified
-
verified
-
verified


Attachments
Test - Both Images Should Be Identical (3.97 KB, image/svg+xml)
2012-02-01 01:30 PST, zscregan
no flags Details
Test - Both Images Should Be Identical (3.97 KB, text/html)
2012-02-01 01:44 PST, zscregan
no flags Details
patch (5.27 KB, patch)
2012-02-01 06:06 PST, Robert Longson
jwatt: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] (still a bit busy) 2012-01-31 14:43:55 PST
Sorry for the vague description...

BUILD: Current nightly, but bug is also present in the Firefox 10 release but NOT in Firefox 9.

STEPS TO REPRODUCE:

1)  Load http://gps.veritrans.ca/beta
2)  Log in with username "Demo" and password "password"
3)  Click the "Time Line" link near the top
4)  When the timeline appears, use your mouse to drag the left-edge indicator
    for the marked blue area.
5)  Then use your mouse to drag the right-edge indicator.

ACTUAL RESULTS:

In step 4, the right-edge indicator jiggles when the left one is dragged.  In step 5, the right-edge indicator itself doesn't move, even though the right edge of the blue area does.  Subsequently dragging the left-edge indicator makes the right-edge one snap to the "right" position where it matches the right edge of the blue area

EXPECTED RESULTS: No weirdness. Dragging the indicators works smoothly and resizes the blue rectangle.

REGRESSION RANGE: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b5b082d183d0&tochange=f25928e4847d

Chances are, a regression from bug 689546 or bug 690486 or bug 614732.
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2012-01-31 14:45:51 PST
In particular, the script on the page is setting the "x" and "width" attributes to values involving percentages on <svg> elements.

My money is on bug 689546.
Comment 2 zscregan 2012-02-01 01:30:49 PST
Created attachment 593338 [details]
Test - Both Images Should Be Identical

I found a workaround to this bug in my web application that I must apply to the url in the description since its used for development and testing.

I created and attached a test case which better tests the bug, please replace this in the description. It appears that the bug is specific to the <use> element when it is positioned relatively, it's parent node is a nested <svg> element, and that <svg> element has it's width changed dynamically.

<svg>
  <svg id="NestedSVG" width="50%">
    <use x="100%" />
  </svg>
</svg>

NestedSVG.setAttribute('width', '75%');
Comment 3 zscregan 2012-02-01 01:44:26 PST
Created attachment 593342 [details]
Test - Both Images Should Be Identical
Comment 4 zscregan 2012-02-01 01:46:16 PST
Comment on attachment 593342 [details]
Test - Both Images Should Be Identical

><!DOCTYPE html>
><html>
>	<head>
>		<title>Nested SVG with Relative Positioning</title>
>		
>		<style>
>			html, body {position:relative; border:0 none; outline:0; padding:0; margin:0; min-width:680px width:100%; min-height:680px; height:100%;}
>			#HTMLContainer {position:absolute; left:20%; right:20%; top:0px; bottom:0px;}
>			#svg1 {position:absolute; top:10px;}
>			#svg2 {position:absolute; top:220px;}
>			#span1 {position:absolute; left:0px; right:0px; top:430px; text-align:center; font-size="18px"}
>		</style>
>	</head>
>	
>	<body>
>		<div id="HTMLContainer">
>			<svg id="svg1" xmlns="http://www.w3.org/2000/svg" version="1.1" width="100%" height="200px">
>				<rect x="0" y="0" width="100%" height="100%" style="fill:rgb(192,128,128); stroke:none" />
>				<svg id="svgNested" x="30%" y="0" width="40%" height="100%">
>					<defs>
>						<polygon id="polygon11" points="20,0 0,10 20,20" style="fill:rgb(255,255,255); stoke:none" />
>						<polygon id="polygon12" points="0,0 0,20 20,10" style="fill:rgb(255,255,255); stoke:none" />
>					</defs>
>				
>					<rect x="0" y="0" width="100%" height="100%" style="fill:rgb(128,128,192); stroke:none" />
>					<rect x="10%" y="94" width="80%" height="12" style="fill:rgb(255,255,255)" />
>					<text x="0" y="14" transform="translate(1,0)" font-size="14px" fill="rgb(255,255,255)">These Images Must Look Identical</text>
>					<text x="100%" y="190" text-anchor="end" transform="translate(-1,0)" font-size="14px" fill="rgb(255,255,255)">These Images Must Look Identical</text>
>					<line x1="0" y1="28" x2="0%" y2="40" transform="translate(5,0)" style="stroke:rgb(255,255,255); stroke-width:2px" />
>					<line x1="100%" y1="172" x2="100%" y2="160" transform="translate(-5,0)" style="stroke:rgb(255,255,255); stroke-width:2px" />
>					<circle cx="0" cy="55" r="10" transform="translate(10,0)" fill="rgb(255,255,255)" />
>					<circle cx="100%" cy="145" r="10" transform="translate(-10,0)" fill="rgb(255,255,255)" />
>					<use x="0" y="70" xlink:href="#polygon11" />
>					<use x="100%" y="110" transform="translate(-20,0)" xlink:href="#polygon12" />
>				</svg>
>			</svg>
>			
>			<svg id="svg2" xmlns="http://www.w3.org/2000/svg" version="1.1" width="100%" height="200px">
>				<defs>
>					<polygon id="polygon21" points="20,0 0,10 20,20" style="fill:rgb(255,255,255); stoke:none" />
>					<polygon id="polygon22" points="0,0 0,20 20,10" style="fill:rgb(255,255,255); stoke:none" />
>				</defs>
>				
>				<rect x="0" y="0" width="100%" height="100%" style="fill:rgb(192,128,128); stroke:none" />
>				<g>
>					<rect x="0" y="0" width="100%" height="100%" style="fill:rgb(128,128,192); stroke:none" />
>					<rect x="10%" y="94" width="80%" height="12" style="fill:rgb(255,255,255)" />
>					<text x="0" y="14" transform="translate(1,0)" font-size="14px" fill="rgb(255,255,255)">These Images Must Look Identical</text>
>					<text x="100%" y="190" text-anchor="end" transform="translate(-1,0)" font-size="14px" fill="rgb(255,255,255)">These Images Must Look Identical</text>
>					<line x1="0%" y1="28" x2="0%" y2="40" transform="translate(5,0)" style="stroke:rgb(255,255,255); stroke-width:2px" />
>					<line x1="100%" y1="172" x2="100%" y2="160" transform="translate(-5,0)" style="stroke:rgb(255,255,255); stroke-width:2px" />
>					<circle cx="0" cy="55" r="10" transform="translate(10,0)" fill="rgb(255,255,255)" />
>					<circle cx="100%" cy="145" r="10" transform="translate(-10,0)" fill="rgb(255,255,255)" />
>					<use x="0" y="70" xlink:href="#polygon21" />
>					<use x="100%" y="110" transform="translate(-20,0)" xlink:href="#polygon22" />
>				</g>
>			</svg>
>			
>			<span id="span1">Please Wait</span>
>		</div>
>		
>		<script>
>			var DOM = {
>				svgNested: document.getElementById('svgNested'),
>				span1: document.getElementById('span1')
>			}
>			
>			function Update() {
>				DOM.svgNested.setAttribute('x', '0');
>				DOM.svgNested.setAttribute('width', '100%');
>				DOM.span1.style.display = 'none';
>			}
>			
>			setTimeout("Update()", 2000);
>		</script>
>	</body>
></html>
Comment 5 Robert Longson 2012-02-01 06:06:29 PST
Created attachment 593419 [details] [diff] [review]
patch
Comment 6 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-02-01 08:05:23 PST
Comment on attachment 593419 [details] [diff] [review]
patch

Thanks Robert.
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2012-02-01 08:31:01 PST
Should we backport this to aurora or beta?
Comment 9 Robert Longson 2012-02-01 09:14:38 PST
You need a <use> or <foreignObject> that has a percentage x or y attribute.
That needs to be a child of an inner <svg> which doesn't have a viewBox attribute but does have width or height attributes that change. 

So, I don't think it's going to be commonly encountered and it looks like the original site has been updated to work around the bug somehow.

On the other hand the patch is rather simple; a couple of tests which set a flag or clear a cached variable so it's a low risk fix with a test.
Comment 10 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-02-01 15:54:21 PST
(In reply to Robert Longson from comment #9)
> You need a <use> or <foreignObject> that has a percentage x or y attribute.
> That needs to be a child of an inner <svg> which doesn't have a viewBox
> attribute but does have width or height attributes that change. 

I wouldn't bet on that being too uncommon myself. And this is really quite a safe patch, since it just makes us do a bit more invalidation when these conditions (common or uncommon) are met.
Comment 11 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-02-01 15:56:40 PST
Comment on attachment 593419 [details] [diff] [review]
patch

[Approval Request Comment]
Regression caused by (bug #): bug 689546
User impact if declined: breaks some SVG content (unclear how much)
Testing completed (on m-c, etc.): landed on m-c
Risk to taking this patch (and alternatives if risky): very low
String changes made by this patch: none
Comment 12 Alex Keybl [:akeybl] 2012-02-02 06:53:40 PST
Comment on attachment 593419 [details] [diff] [review]
patch

[Triage Comment]
Given the low-risk nature of the patch and the fact that we think this may be happening in the wild, approving for Aurora 12 and Beta 11.
Comment 13 Robert Longson 2012-02-02 06:57:28 PST
Jonathan, can you land this on beta and aurora please?
Comment 14 Ed Morley [:emorley] 2012-02-02 08:08:06 PST
https://hg.mozilla.org/mozilla-central/rev/452e9dfec0e2
Comment 15 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-02-02 08:50:38 PST
Sure thing.

Landed https://tbpl.mozilla.org/?tree=Mozilla-Beta&rev=1a62ea0a1fcc

Will land on m-a later.
Comment 16 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-02-03 03:15:03 PST
Sorry, Beta link should be:

https://hg.mozilla.org/releases/mozilla-beta/rev/1a62ea0a1fcc

Also landed on Aurora now:

https://hg.mozilla.org/releases/mozilla-aurora/rev/7d6a23398033
Comment 17 Teodosia Pop 2012-02-10 06:22:31 PST
Verified fixed using Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0) Gecko/20100101 Firefox/11.0.
Comment 18 Teodosia Pop 2012-02-10 07:34:54 PST
Verified fixed using on Linux and Mac OS X:
Mozilla/5.0 (X11; Linux i686; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0
Comment 19 Virgil Dicu [:virgil] [QA] 2012-02-10 09:27:58 PST
Mozilla/5.0 (X11; Linux x86_64; rv:12.0a2) Gecko/20120210 Firefox/12.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:13.0a1) Gecko/20120210 Firefox/13.0a1
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:12.0a2) Gecko/20120210 Firefox/12.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:13.0a1) Gecko/20120208 Firefox/13.0a1
Mozilla/5.0 (Windows NT 6.1; rv:12.0a2) Gecko/20120210 Firefox/12.0a2
Mozilla/5.0 (Windows NT 6.1; rv:13.0a1) Gecko/20120210 Firefox/13.0a1

Used testcase to verify. Changing status to verified on F12 and F13.

Note You need to log in before you can comment on or make changes to this bug.