Bug 63109 – SVG1.1SE test with pointer-events and invalid gradient fill fails

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Rob Buis]

This test fails because the top rect reports it is hit where it shouldn't.

[Rob Buis]

Created attachment 98092 [details]
Patch

[WebKit Review Bot]

Comment on attachment 98092 [details]
Patch

Attachment 98092 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8925122

New failing tests:
svg/W3C-SVG-1.1-SE/interact-pointer-03-t.svg

[WebKit Review Bot]

Created attachment 98095 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Nikolas Zimmermann]

Comment on attachment 98092 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=98092&action=review

Great job! Almost r+, though we need an automatized test...

> Source/WebCore/css/SVGCSSParser.cpp:194
> +                if (m_valueList->next()) {
> +                    if (parseColorFromValue(m_valueList->current(), c))
> +                        parsedValue = SVGPaint::createURIAndColor(value->string, c);
> +                    else if (m_valueList->current()->id == CSSValueNone)
> +                        parsedValue = SVGPaint::createURIAndNone(value->string);
> +                }

Excellent catch!

> LayoutTests/svg/W3C-SVG-1.1-SE/interact-pointer-03-t.svg:46
> +        <font-face-uri xlink:href="../resources/SVGFreeSans.svg#ascii"/>

Is this link correct? Please grep for SVGFreeSans.svg in LayoutTests/svg/W3C-SVG-1.1-SE - you need to alter the path.

> LayoutTests/svg/W3C-SVG-1.1-SE/interact-pointer-03-t.svg:55
> +    <rect id='r2' x='10' y='80' width='50' height='50' clip-path='url(#c)' fill='url(#invalid) none' pointer-events='painted' onmouseover='report(false)'/>

I guess this new test turns green now on hovering?
If so, we need another testcase that checks your new code w/o doing hovering etc. otherwhise your change won't be backed by any test run in DRT.

[Rob Buis]

Created attachment 98162 [details]
Patch

[Rob Buis]

Moin Niko,

(In reply to comment #4)
> > LayoutTests/svg/W3C-SVG-1.1-SE/interact-pointer-03-t.svg:46
> > +        <font-face-uri xlink:href="../resources/SVGFreeSans.svg#ascii"/>
> 
> Is this link correct? Please grep for SVGFreeSans.svg in LayoutTests/svg/W3C-SVG-1.1-SE - you need to alter the path.

It was wrong, I corrected it now.

> > LayoutTests/svg/W3C-SVG-1.1-SE/interact-pointer-03-t.svg:55
> > +    <rect id='r2' x='10' y='80' width='50' height='50' clip-path='url(#c)' fill='url(#invalid) none' pointer-events='painted' onmouseover='report(false)'/>
> 
> I guess this new test turns green now on hovering?
> If so, we need another testcase that checks your new code w/o doing hovering etc. otherwhise your change won't be backed by any test run in DRT.

I made this a text-only test and tried to make as few changes as possible to the original. Let me know if I should remove more to make the text result file even smaller. Also I guess we will need to do this for any 1.1SE test we add? We didn't do it for 1.1 testsuite, and I guess it was kind of useless to include results for the interactive ones....
Cheers,

Rob.

[Nikolas Zimmermann]

Comment on attachment 98162 [details]
Patch

I think the idea is to move your modified test to eg svg/custom/a-meaningful-name-for-this-test.svg, and leave the interact-pointer-03-t.svg as-is in svg/W3C-SVG-1.1-SE/. We can immediately tell which tests we fixed from SVG 1.1 2nd Edition, by looking into svg/W3C-SVG-1.1-SE/. Each test present there works and if its interactive, it has a test that excersises the dynamic parts somewhere else (in eg. svg/custom for your case). Sounds reasonable?

I'm going to set r+, as the patch is fine, but please place the original W3C test in svg/W3C-SVG-1.1-SE/ and your modified one in svg/custom/pointer-events-invalid-fill.svg (for example).

[Rob Buis]

Hi Niko,

(In reply to comment #7)
> (From update of attachment 98162 [details])
> I think the idea is to move your modified test to eg svg/custom/a-meaningful-name-for-this-test.svg, and leave the interact-pointer-03-t.svg as-is in svg/W3C-SVG-1.1-SE/. We can immediately tell which tests we fixed from SVG 1.1 2nd Edition, by looking into svg/W3C-SVG-1.1-SE/. Each test present there works and if its interactive, it has a test that excersises the dynamic parts somewhere else (in eg. svg/custom for your case). Sounds reasonable?

I am fine with that but what about the test results of the test in W3C-SVG-1-1-SE? 

> I'm going to set r+, as the patch is fine, but please place the original W3C test in svg/W3C-SVG-1.1-SE/ and your modified one in svg/custom/pointer-events-invalid-fill.svg (for example).

Thanks for the r+!
Cheers,

Rob.

[Rob Buis]

Committed r89490: <http://trac.webkit.org/changeset/89490>