Bug 153854 – Allow href attribute without xlink on SVG elements

• Component: SVG
• Status: RESOLVED FIXED

[[e-mail]]

SVG Working group has resolved to deprecate xlink and allow href attribute on relevant nodes (https://www.w3.org/2012/09/19-svg-minutes.html#item05), the according spec changes has been made (https://svgwg.org/svg2-draft/attindex.html), and this has been brought to attention today during Sydney face to face meeting - https://www.w3.org/2016/02/03-svg-minutes.html

At this point Edge supports that, Chrome doesn't.

Simple test case: https://jsfiddle.net/pzrez3eb/

[[e-mail]]

Copied from https://code.google.com/p/chromium/issues/detail?id=584142

[Cameron McCormack (:heycam)]

FYI, we just landed support for this in Firefox, and Chrome has supported this since February this year.

[Radar WebKit Bug Importer]

<rdar://problem/28116759>

[Cigitia]

I ran into this problem today. All major browser engines except WebKit now support plain href in SVG. This includes Firefox, Chrome/Opera, Edge, and even Internet Explorer (https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/href). With the multiyear-old deprecation of xlink:href in SVG 2.0, this is a pernicious web-compatibility issue. Thanks in advance to anyone who resolves this bug.

[Said Abou-Hallawa]

*** Bug 186315 has been marked as a duplicate of this bug. ***

[Said Abou-Hallawa]

Created attachment 346680 [details]
Patch

[EWS Watchlist]

Comment on attachment 346680 [details]
Patch

Attachment 346680 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/8783756

New failing tests:
fast/selectors/any-link-basics-2.html

[EWS Watchlist]

Created attachment 346685 [details]
Archive of layout-test-results from ews115 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 346680 [details]
Patch

Attachment 346680 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/8784674

New failing tests:
fast/selectors/any-link-basics-2.html

[EWS Watchlist]

Created attachment 346690 [details]
Archive of layout-test-results from ews113 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Simon Fraser (smfr)]

Comment on attachment 346680 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        SVG 2 removed the need for the xlink namespace, so instead of xlink:href
> +        href should be used. See https://www.w3.org/TR/SVG2/linking.html#XLinkRefAttrs.

Is this behavior independent of the xmlns attribute on the <svg> element, or are SVG 2 documents in a mode where bare "href" is allowed?

[Said Abou-Hallawa]

Created attachment 346714 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 346680 [details]
Patch

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

>> Source/WebCore/ChangeLog:9
>> +        href should be used. See https://www.w3.org/TR/SVG2/linking.html#XLinkRefAttrs.
> 
> Is this behavior independent of the xmlns attribute on the <svg> element, or are SVG 2 documents in a mode where bare "href" is allowed?

The href attribute now is part of the svg namespace. If the attribute xmlns="http://www.w3.org/2000/svg" is added to the <svg> element, then you can write "href" instead of "svg:href".

[Said Abou-Hallawa]

Created attachment 346725 [details]
Patch

[Dean Jackson]

Comment on attachment 346725 [details]
Patch

Why not add some reference tests that exercise references to filters, patterns, images, use, anchors - with and without the xlink: prefix? I'd definitely like to see a test that <a> works.

[Said Abou-Hallawa]

Created attachment 346742 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 346742 [details]
Patch

Clearing flags on attachment: 346742

Committed r234683: <https://trac.webkit.org/changeset/234683>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 346742 [details]
Patch

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

> Source/WebCore/ChangeLog:32
> +        (WebCore::Element::getAttribute const): This new template function with
> +        template pack parameter QualifiedNames is added to return the first none
> +        empty attribute value given a set of attributes' names. This should be
> +        useful for deprecated attributes. When we decide to remove the support 
> +        for the deprecated attribute, all we need is to remove it as a parameter
> +        to getAttribute(). In this case, the none template function will be called.

I would have expected this to return the first non-null attribute value, not the first non-empty. Typically we would not ignore an attribute with an empty value.

Do we have test cases covering the behavior of ignoring an empty attribute? Is it truly the behavior we want for this first use of the new getAttribute function?

I’d like us to follow up and change it to non-null unless there is a reason the non-empty behavior is preferred.

[Said Abou-Hallawa]

Comment on attachment 346742 [details]
Patch

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

>> Source/WebCore/ChangeLog:32
>> +        to getAttribute(). In this case, the none template function will be called.
> 
> I would have expected this to return the first non-null attribute value, not the first non-empty. Typically we would not ignore an attribute with an empty value.
> 
> Do we have test cases covering the behavior of ignoring an empty attribute? Is it truly the behavior we want for this first use of the new getAttribute function?
> 
> I’d like us to follow up and change it to non-null unless there is a reason the non-empty behavior is preferred.

The followup bug is https://bugs.webkit.org/show_bug.cgi?id=188419.

[Said Abou-Hallawa]

*** Bug 183748 has been marked as a duplicate of this bug. ***

[Said Abou-Hallawa]

*** Bug 181283 has been marked as a duplicate of this bug. ***