862158 - tspan-rotate-02.svg and tspan-rotate-04.svg fail with svg.text.css-frames.enabled

Status: RESOLVED FIXED

(Core :: SVG, defect)

Comment 1

[Cameron McCormack (:heycam)]

Attachment: patch

This keeps http://www.w3.org/Graphics/SVG/Test/20110816/svg/text-tspan-02-b.svg passing while making tspan-0[24].svg pass.  I also added a test for unspecified rotation values that come after specified values but which have nothing to inherit from.  Other implementations (and our current old SVG text frames) use 0 rather than looking at the preceding value.

Comment 2

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/66c01cdf30d2

Comment 3

[Cameron McCormack (:heycam)]

Backed out for text-layout-08.svg failure:
https://hg.mozilla.org/integration/mozilla-inbound/rev/00c31be62f62

Comment 4

[Cameron McCormack (:heycam)]

Attachment: patch v2

I'm sorry, I got this one wrong.  In fact, we do want to overwrite inherited rotation values, even if they were explicitly specified.  Reading through the description of rotate="" in SVG 1.1 Second Edition (and SVG 2, still) this is what's required.  I remember that there was an errata that made its way into the Second Edition.

This means we don't have to track whether the inherited rotation value was implied or not.  It also means that text-tspan-0[24].svg are invalid as they are currently.  In this patch I've updated them to the new behaviour.  This makes us match Chrome's behaviour for these tests.

Try run for this new patch in progress: https://tbpl.mozilla.org/?tree=Try&rev=f9e9154e1cd4

Comment 5

[Robert Longson [:longsonr]]

Comment on attachment 737928 [details] [diff] [review]
patch v2

> skip-if(B2G) random-if(/^Windows\x20NT\x205\.1/.test(http.oscpu)) == tspan-rotate-01.svg tspan-rotate-ref.svg # Bug 629718 for random failure on WinXP # bug 773482
>-skip-if(B2G) random-if(/^Windows\x20NT\x205\.1/.test(http.oscpu)) == tspan-rotate-02.svg tspan-rotate-ref.svg # Bug 629718 for random failure on WinXP # bug 773482
>+skip-if(B2G) random-if(/^Windows\x20NT\x205\.1/.test(http.oscpu)) pref(svg.text.css-frames.enabled,true) == tspan-rotate-02.svg tspan-rotate-02-ref.svg # Bug 629718 for random failure on WinXP # bug 773482
> skip-if(B2G) random-if(/^Windows\x20NT\x205\.1/.test(http.oscpu)) == tspan-rotate-03.svg tspan-rotate-ref.svg # Bug 629718 for random failure on WinXP # bug 773482
>-skip-if(B2G) random-if(/^Windows\x20NT\x205\.1/.test(http.oscpu)) == tspan-rotate-04.svg tspan-rotate-ref.svg # Bug 629718 for random failure on WinXP # bug 773482
>+skip-if(B2G) random-if(/^Windows\x20NT\x205\.1/.test(http.oscpu)) pref(svg.text.css-

I'm not keen on the changes above, we could regress the current shipping functionality and not know it. Lets just leave this as it is. Once we pref this on it will run anyway.

>+== tspan-rotate-07.svg tspan-rotate-07-ref.svg

Please run this on Try on Windows XP a few times to check you're not introducing random orange. If you are skip the test on WinXP.

r=me with above issues fixed.

Comment 6

[Cameron McCormack (:heycam)]

(In reply to Robert Longson from comment #5)
> I'm not keen on the changes above, we could regress the current shipping
> functionality and not know it. Lets just leave this as it is. Once we pref
> this on it will run anyway.

Should I duplicate the files then and have one version for current functionality and one for new?  If I make the test changes but leave them running without the pref, they'll fail.

> >+== tspan-rotate-07.svg tspan-rotate-07-ref.svg
> 
> Please run this on Try on Windows XP a few times to check you're not
> introducing random orange. If you are skip the test on WinXP.

OK.

Comment 7

[Cameron McCormack (:heycam)]

Win XP try run kicked off: https://tbpl.mozilla.org/?tree=Try&rev=6c8099ec6f95

Comment 8

[Robert Longson [:longsonr]]

Comment on attachment 737928 [details] [diff] [review]
patch v2

was supposed to have test changes in to tspan-rotate-02.svg and tspan-rotate-04.svg

Comment 9

[Cameron McCormack (:heycam)]

Attachment: patch v2.1

This time with missing files added.  I duplicated the reftest.list lines for these two tests so we don't lose test coverage.

Comment 10

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/62867a7d2d44

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/62867a7d2d44