| Summary: | Drop support for non-standard document.all.tags() | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||
| Component: | DOM | Assignee: | Chris Dumez <cdumez> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | achristensen, buildbot, cdumez, commit-queue, darin, dbates, esprehn+autocc, gyuyoung.kim, kondapallykalyan, rniwa, sam | ||||||||
| Priority: | P2 | Keywords: | WebExposed | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Chris Dumez
2017-03-07 20:21:42 PST
Created attachment 303767 [details]
Patch
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 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
Created attachment 303778 [details]
Patch
document.all.tags() is a proprietary Microsoft extension and is supported in IE 11. 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? (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. (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() (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? (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. 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. (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. (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. Given the results on Edge, can I now land this patch? (In reply to comment #14) > Given the results on Edge, can I now land this patch? I double r+. (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? (In reply to comment #14) > Given the results on Edge, can I now land this patch? OK Comment on attachment 303778 [details] Patch Clearing flags on attachment: 303778 Committed r213619: <http://trac.webkit.org/changeset/213619> All reviewed patches have been landed. Closing bug. |