Bug 201227 – XLinkNames namespace is required before the 'href' attribute of SVG animate elements

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

[Said Abou-Hallawa]

Open the attached test case.

Result: One red rectangle and one green rectangle.
Expected: Tow green rectangles.

In the test case there are two SVG <rect> elements. The two <rect> elements are referenced by two <set> elements to change their "fill" attribute from "red" to "green". The first <set> element has the attribute "href" set to "#rect". The second <set> element has the attribute "xlink:href" set to "#rect2". The first <set> element fails to set its target correctly while the second succeeds to set its target correctly.

This is a left over work from r234683.

[Said Abou-Hallawa]

Created attachment 377460 [details]
test case

[Said Abou-Hallawa]

Created attachment 377461 [details]
Patch

[Said Abou-Hallawa]

Created attachment 377467 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 377467 [details]
Patch

Clearing flags on attachment: 377467

Committed r249216: <https://trac.webkit.org/changeset/249216>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/54803998>

[Darin Adler]

Comment on attachment 377467 [details]
Patch

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

> LayoutTests/svg/custom/href-svg-namespace-animate-target.svg:2
> +    <desc>Verify the XLinkNames namespace is not requried before the 'href' attribute of the SVG animate elements.</desc>

Typoe "requried".

Also, is <desc> the best way to add comments to SVG? Since this ends up just serving as a comment.

[Darin Adler]

Comment on attachment 377467 [details]
Patch

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

> LayoutTests/svg/custom/href-svg-namespace-animate-target.svg:6
> +    <rect id="rect1" width="100" height="100" fill="red"/>
> +    <set id="set" attributeName="fill" attributeType="CSS" href="#rect1"  to="green" begin="0s" fill="freeze"/> 
> +    <rect id="rect2" x="110" width="100" height="100" fill="red"/>
> +    <set id="set" attributeName="fill" attributeType="CSS" xlink:href="#rect2"  to="green" begin="0s" fill="freeze"/> 

I think we should consider more test coverage: tests that check which attribute is respected if multiple are present in various orders. The code currently will prioritize href over xlink:href no matter what order the attributes appear in, and I’d like to see tests that check that because those are the kinds of things that can become interoperability problems between web browsers. I’d also be interested in at least some tests that check if the elements respond properly if the attributes are changed dynamically by script.

[Said Abou-Hallawa]

Comment on attachment 377467 [details]
Patch

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

>> LayoutTests/svg/custom/href-svg-namespace-animate-target.svg:2
>> +    <desc>Verify the XLinkNames namespace is not requried before the 'href' attribute of the SVG animate elements.</desc>
> 
> Typoe "requried".
> 
> Also, is <desc> the best way to add comments to SVG? Since this ends up just serving as a comment.

I will fix the typo in a followup patch.

>> LayoutTests/svg/custom/href-svg-namespace-animate-target.svg:6
>> +    <set id="set" attributeName="fill" attributeType="CSS" xlink:href="#rect2"  to="green" begin="0s" fill="freeze"/> 
> 
> I think we should consider more test coverage: tests that check which attribute is respected if multiple are present in various orders. The code currently will prioritize href over xlink:href no matter what order the attributes appear in, and I’d like to see tests that check that because those are the kinds of things that can become interoperability problems between web browsers. I’d also be interested in at least some tests that check if the elements respond properly if the attributes are changed dynamically by script.

This is true. We are not compliant with the specs in what to be respected when both href and xlink:href exist. For example, this SVG will display a red circle in WebKit although it should show a green rectangle:

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
    <defs>
        <circle id="circle" cx="50" cy="50" r="50" fill="red"/>
        <rect id="rect" width="100" height="100" fill="green"/>
    </defs>
    <use href="#rect" xlink:href="#circle"/>
</svg>

This issue was reported in https://bugs.webkit.org/show_bug.cgi?id=195802 which I am going to use to address your comments and to ensure we are compliant with the specs.

[Said Abou-Hallawa]

(In reply to Darin Adler from comment #7)
> Comment on attachment 377467 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=377467&action=review
> 
> > LayoutTests/svg/custom/href-svg-namespace-animate-target.svg:2
> > +    <desc>Verify the XLinkNames namespace is not requried before the 'href' attribute of the SVG animate elements.</desc>
> 
> Typoe "requried".
> 
> Also, is <desc> the best way to add comments to SVG? Since this ends up just
> serving as a comment.

The typo and the <desc> issue were fixed in <https://trac.webkit.org/changeset/249579>.