Bug 153854

Summary: Allow href attribute without xlink on SVG elements
Product: WebKit Reporter: bbrinza <bbrinza>
Component: SVGAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Minor CC: cigitia, commit-queue, darin, dino, dvpdiner2, eoconnor, ews-watchlist, heycam, krit, sabouhallawa, shayneosull, simon.fraser, webkit.20.phk, webkit-bug-importer, zach.silversmith, zimmermann
Priority: P2 Keywords: FromImplementor, InRadar
Version: Safari 9   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=188419
https://bugs.webkit.org/show_bug.cgi?id=201227
Bug Depends on:    
Bug Blocks: 191292    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews115 for mac-sierra
none
Archive of layout-test-results from ews113 for mac-sierra
none
Patch
none
Patch
none
Patch none

bbrinza@microsoft.com
Reported 2016-02-03 20:31:22 PST
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/
Attachments
Patch (19.38 KB, patch)
2018-08-06 20:19 PDT, Said Abou-Hallawa 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from ews115 for mac-sierra (3.02 MB, application/zip)
2018-08-06 22:25 PDT, EWS Watchlist 		no flags
Details
Archive of layout-test-results from ews113 for mac-sierra (3.04 MB, application/zip)
2018-08-07 00:02 PDT, EWS Watchlist 		no flags
Details
Patch (26.31 KB, patch)
2018-08-07 10:37 PDT, Said Abou-Hallawa 		no flags
DetailsFormatted DiffDiff
Patch (35.34 KB, patch)
2018-08-07 13:25 PDT, Said Abou-Hallawa 		no flags
DetailsFormatted DiffDiff
Patch (45.33 KB, patch)
2018-08-07 16:33 PDT, Said Abou-Hallawa 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
bbrinza@microsoft.com
Comment 1 2016-02-03 20:31:58 PST
Copied from https://code.google.com/p/chromium/issues/detail?id=584142
Cameron McCormack (:heycam)
Comment 2 2016-08-31 20:27:14 PDT
FYI, we just landed support for this in Firefox, and Chrome has supported this since February this year.
Radar WebKit Bug Importer
Comment 3 2016-09-01 09:38:52 PDT
<rdar://problem/28116759>
Cigitia
Comment 4 2017-12-04 19:40:46 PST
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
Comment 5 2018-06-05 12:37:33 PDT
*** Bug 186315 has been marked as a duplicate of this bug. ***
Said Abou-Hallawa
Comment 6 2018-08-06 20:19:20 PDT
Created attachment 346680 [details] Patch
EWS Watchlist
Comment 7 2018-08-06 22:25:48 PDT
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
Comment 8 2018-08-06 22:25:49 PDT
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 9 2018-08-07 00:02:41 PDT
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
Comment 10 2018-08-07 00:02:43 PDT
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 11 2018-08-07 08:31:25 PDT
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
Comment 12 2018-08-07 10:37:03 PDT
Created attachment 346714 [details] Patch
Said Abou-Hallawa
Comment 13 2018-08-07 10:40:18 PDT
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
Comment 14 2018-08-07 13:25:42 PDT
Created attachment 346725 [details] Patch
Dean Jackson
Comment 15 2018-08-07 13:40:04 PDT
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
Comment 16 2018-08-07 16:33:38 PDT
Created attachment 346742 [details] Patch
WebKit Commit Bot
Comment 17 2018-08-07 17:55:58 PDT
Comment on attachment 346742 [details] Patch Clearing flags on attachment: 346742 Committed r234683: <https://trac.webkit.org/changeset/234683>
WebKit Commit Bot
Comment 18 2018-08-07 17:56:00 PDT
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 19 2018-08-08 09:13:44 PDT
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 20 2018-08-08 14:31:08 PDT
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
Comment 21 2019-05-06 15:33:52 PDT
*** Bug 183748 has been marked as a duplicate of this bug. ***
Said Abou-Hallawa
Comment 22 2019-05-07 09:16:51 PDT
*** Bug 181283 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.