RESOLVED FIXED Bug 169337
Drop support for non-standard document.all.tags()
https://bugs.webkit.org/show_bug.cgi?id=169337
Summary Drop support for non-standard document.all.tags()
Chris Dumez
Reported 2017-03-07 20:21:42 PST
Drop support for non-standard document.all.tags(). It is not part of the specification: - https://html.spec.whatwg.org/multipage/infrastructure.html#the-htmlallcollection-interface It is not supported by Firefox and its support was dropped from Chrome back in early 2014: - https://src.chromium.org/viewvc/blink?view=revision&revision=166870
Attachments
Patch (7.01 KB, patch)
2017-03-07 20:24 PST, Chris Dumez
no flags
Archive of layout-test-results from ews105 for mac-elcapitan-wk2 (954.44 KB, application/zip)
2017-03-07 21:10 PST, Build Bot
no flags
Patch (8.15 KB, patch)
2017-03-07 21:24 PST, Chris Dumez
no flags
Chris Dumez
Comment 1 2017-03-07 20:24:13 PST
Build Bot
Comment 2 2017-03-07 21:10:08 PST
Comment on attachment 303767 [details] Patch Attachment 303767 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3263329 New failing tests: fast/dom/HTMLDocument/document-all.html
Build Bot
Comment 3 2017-03-07 21:10:12 PST
Created attachment 303774 [details] Archive of layout-test-results from ews105 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Chris Dumez
Comment 4 2017-03-07 21:24:16 PST
Daniel Bates
Comment 5 2017-03-08 09:15:17 PST
document.all.tags() is a proprietary Microsoft extension and is supported in IE 11.
Daniel Bates
Comment 6 2017-03-08 09:17:27 PST
Is the Google UseCounter data referenced in the commit message for <https://src.chromium.org/viewvc/blink?view=revision&revision=166870> an accurate representation of enterprise customers?
Chris Dumez
Comment 7 2017-03-08 09:58:16 PST
(In reply to comment #5) > document.all.tags() is a proprietary Microsoft extension and is supported in > IE 11. https://msdn.microsoft.com/library/bg182625(v=vs.85).aspx#legacyAPIs says that document.all was dropped in IE11? I do not have a virtual machine handy to verify. Either way, I think Edge matters a lot more than IE11 at this point. I was able to remove document.all.tags from Blink 3 years ago. There were no complaints about breaking after removing it. So even if you believe UseCounter data is not necessarily indicative of enterprise use, I believe the fact that there were no complaints after removing it is good indication that the risk should be low and that it is at least worth trying. Note that when showModalDialog was dropped in Blink for example, there *were* a lot of complaints from enterprise users.
Daniel Bates
Comment 8 2017-03-08 10:21:08 PST
(In reply to comment #7) > (In reply to comment #5) > > document.all.tags() is a proprietary Microsoft extension and is supported in > > IE 11. > > https://msdn.microsoft.com/library/bg182625(v=vs.85).aspx#legacyAPIs says > that document.all was dropped in IE11? I do not have a virtual machine handy > to verify. It was not removed at least in IE 11.0.9600.18500. It would be good to know what version of IE11 removes it? > Either way, I think Edge matters a lot more than IE11 at this point. > > I was able to remove document.all.tags from Blink 3 years ago. There were no > complaints about breaking after removing it. That is good to know though it assumes that such breakage would be reported. I suspect that usage of document.all.tags() would be on unmaintained legacy web pages, including cached pages (say, archive.org). > So even if you believe > UseCounter data is not necessarily indicative of enterprise use, I believe > the fact that there were no complaints after removing it is good indication > that the risk should be low and that it is at least worth trying. > > Note that when showModalDialog was dropped in Blink for example, there > *were* a lot of complaints from enterprise users. And ... What is the benefit of removing document.all.tags()? Do we know of any modern web pages (in the last 3 years) whose behavior changes by removing document.all.tags()
Daniel Bates
Comment 9 2017-03-08 10:26:05 PST
(In reply to comment #8) > What is the benefit of removing document.all.tags()? Obviously it benefits us (the OpenSource WebKit project) as it removes 11 lines of code (includes whitespace). Does it allow us to remove more code?
Chris Dumez
Comment 10 2017-03-08 10:27:04 PST
(In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #5) > > > document.all.tags() is a proprietary Microsoft extension and is supported in > > > IE 11. > > > > https://msdn.microsoft.com/library/bg182625(v=vs.85).aspx#legacyAPIs says > > that document.all was dropped in IE11? I do not have a virtual machine handy > > to verify. > > It was not removed at least in IE 11.0.9600.18500. It would be good to know > what version of IE11 removes it? > > > Either way, I think Edge matters a lot more than IE11 at this point. > > > > I was able to remove document.all.tags from Blink 3 years ago. There were no > > complaints about breaking after removing it. > > That is good to know though it assumes that such breakage would be reported. > I suspect that usage of document.all.tags() would be on unmaintained legacy > web pages, including cached pages (say, archive.org). > > > So even if you believe > > UseCounter data is not necessarily indicative of enterprise use, I believe > > the fact that there were no complaints after removing it is good indication > > that the risk should be low and that it is at least worth trying. > > > > Note that when showModalDialog was dropped in Blink for example, there > > *were* a lot of complaints from enterprise users. > > And ... > > What is the benefit of removing document.all.tags()? Do we know of any > modern web pages (in the last 3 years) whose behavior changes by removing > document.all.tags() The benefit is to align with other browsers and the specification (and drop some code in the process, although not a lot). Other browsers are aligning with standards and we should do the same. We do not want new content developed in WebKit to not work in other browsers because it relies on document.all.tags. Note that document.all is already deprecated and we already have mitigations in place to make it less likely do be used (e.g. document.all == undefined) in the hope of removing it completely some day. This is a step in this direction.
Daniel Bates
Comment 11 2017-03-08 10:39:57 PST
I know this patch was r+'ed. I am typing some remarks and questions that I would prefer to be answered before this patch is landed.
Daniel Bates
Comment 12 2017-03-08 11:11:22 PST
(In reply to comment #10) If this extension is removed from the latest version of IE11 then it seems reasonable to remove it from WebKit. Otherwise, is there harm of keeping this extension? I mean, it does not seem very intrusive to the codebase as it is effectively an alias for document.getElementsByTagName() (though maybe there is a cost to keep some form of document.all working?) As far as I am aware document.all.tags() is a proprietary Microsoft extension and hence never standardized. I take it there is little value in concerning ourself with supporting unmaintained legacy web pages or content archived in archive.org? > [...] > The benefit is to align with other browsers and the specification (and drop > some code in the process, although not a lot). Other browsers are aligning > with standards and we should do the same. > We do not want new content > developed in WebKit to not work in other browsers because it relies on > document.all.tags. Are we actually worried about this? I mean, document.all.tags() in not listed in the standard.
Chris Dumez
Comment 13 2017-03-08 12:57:54 PST
(In reply to comment #12) > (In reply to comment #10) > > If this extension is removed from the latest version of IE11 then it seems > reasonable to remove it from WebKit. Otherwise, is there harm of keeping > this extension? I mean, it does not seem very intrusive to the codebase as > it is effectively an alias for document.getElementsByTagName() (though maybe > there is a cost to keep some form of document.all working?) > > As far as I am aware document.all.tags() is a proprietary Microsoft > extension and hence never standardized. I take it there is little value in > concerning ourself with supporting unmaintained legacy web pages or content > archived in archive.org? This proprietary Microsoft extension is definitely gone in Edge, I have just verified this in a VM. I do not see much point in keeping a Microsoft extension that even Microsoft has dropped. > > > [...] > > The benefit is to align with other browsers and the specification (and drop > > some code in the process, although not a lot). Other browsers are aligning > > with standards and we should do the same. > > We do not want new content > > developed in WebKit to not work in other browsers because it relies on > > document.all.tags. > > Are we actually worried about this? I mean, document.all.tags() in not > listed in the standard. As long as an API is exposed in our engine, there are chances people will use them. And we do care about aligning with other browsers and the standards. I care about this a lot more than trying to maintain an old IE extension that even Microsoft dropped in their latest product.
Chris Dumez
Comment 14 2017-03-08 14:27:26 PST
Given the results on Edge, can I now land this patch?
Sam Weinig
Comment 15 2017-03-08 16:27:40 PST
(In reply to comment #14) > Given the results on Edge, can I now land this patch? I double r+.
Daniel Bates
Comment 16 2017-03-08 18:16:46 PST
(In reply to comment #13) > (In reply to comment #12) > > (In reply to comment #10) > > > > If this extension is removed from the latest version of IE11 then it seems > > reasonable to remove it from WebKit. Otherwise, is there harm of keeping > > this extension? I mean, it does not seem very intrusive to the codebase as > > it is effectively an alias for document.getElementsByTagName() (though maybe > > there is a cost to keep some form of document.all working?) > > > > As far as I am aware document.all.tags() is a proprietary Microsoft > > extension and hence never standardized. I take it there is little value in > > concerning ourself with supporting unmaintained legacy web pages or content > > archived in archive.org? > > This proprietary Microsoft extension is definitely gone in Edge, I have just > verified this in a VM. I do not see much point in keeping a Microsoft > extension that even Microsoft has dropped. > I would expect this to be removed from Edge. From my understanding IE11 is still supported and shipped by Microsoft for backwards compatibility and Edge is their new browser that intentionally breaks with backwards compatibility to be more conformant with other browsers and standards. > > > > > [...] > > > The benefit is to align with other browsers and the specification (and drop > > > some code in the process, although not a lot). Other browsers are aligning > > > with standards and we should do the same. > > > We do not want new content > > > developed in WebKit to not work in other browsers because it relies on > > > document.all.tags. > > > > Are we actually worried about this? I mean, document.all.tags() in not > > listed in the standard. > > As long as an API is exposed in our engine, there are chances people will > use them. We could log a deprecation warning or restrict usage of it to quirks mode. > And we do care about aligning with other browsers and the standards. I care > about this a lot more than trying to maintain an old IE extension that even > Microsoft dropped in their latest product. I think the focus should be on improving the user experience and avoiding breakage of saved web archives of old pages or unmaintained legacy web pages. If an improvement to the user experience can be accomplished by aligning with other browsers and standards that is a great. If not, we should either work with the standards body or weigh the pros/cons of a change with the impact to the user experience. From this discussion it seems we are more concerned with matching the standard. Maybe this is acceptable?
Daniel Bates
Comment 17 2017-03-08 18:17:04 PST
(In reply to comment #14) > Given the results on Edge, can I now land this patch? OK
Chris Dumez
Comment 18 2017-03-08 18:37:47 PST
Comment on attachment 303778 [details] Patch Clearing flags on attachment: 303778 Committed r213619: <http://trac.webkit.org/changeset/213619>
Chris Dumez
Comment 19 2017-03-08 18:37:56 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.