Bug 173444 – [WebIDL] Remove custom bindings for HTMLDocument

• Component: New Bugs
• Status: RESOLVED FIXED

[Sam Weinig]

[WebIDL] Remove custom bindings for HTMLDocument

[Sam Weinig]

Created attachment 313020 [details]
Patch

[Build Bot]

Comment on attachment 313020 [details]
Patch

Attachment 313020 [details] did not pass bindings-ews (mac):
Output: http://webkit-queues.webkit.org/results/3938085

New failing tests:
(JS) JSTestObj.cpp

[Sam Weinig]

Created attachment 313021 [details]
Patch

[Sam Weinig]

Created attachment 313029 [details]
Patch

[Build Bot]

Comment on attachment 313029 [details]
Patch

Attachment 313029 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3938513

New failing tests:
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/opening-the-input-stream/document.open-02.html
http/tests/security/isolatedWorld/document-open.html
fast/dom/frame-deleted-in-document-open.html

[Build Bot]

Created attachment 313037 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 313029 [details]
Patch

Attachment 313029 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3938539

New failing tests:
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/opening-the-input-stream/document.open-02.html
http/tests/security/isolatedWorld/document-open.html
fast/dom/frame-deleted-in-document-open.html

[Build Bot]

Created attachment 313038 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 313029 [details]
Patch

Attachment 313029 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3938573

New failing tests:
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/opening-the-input-stream/document.open-02.html
http/tests/security/isolatedWorld/document-open.html
fast/dom/frame-deleted-in-document-open.html

[Build Bot]

Created attachment 313043 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Build Bot]

Comment on attachment 313029 [details]
Patch

Attachment 313029 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3938567

New failing tests:
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/opening-the-input-stream/document.open-02.html
http/tests/security/isolatedWorld/document-open.html
fast/dom/frame-deleted-in-document-open.html

[Build Bot]

Created attachment 313045 [details]
Archive of layout-test-results from ews113 for mac-elcapitan

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

[Sam Weinig]

Created attachment 313081 [details]
Patch

[Build Bot]

Comment on attachment 313081 [details]
Patch

Attachment 313081 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3942290

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 313087 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 313081 [details]
Patch

Attachment 313081 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3942294

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 313088 [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

[Sam Weinig]

Created attachment 313092 [details]
Patch

[Sam Weinig]

Created attachment 313097 [details]
Patch

[Build Bot]

Comment on attachment 313097 [details]
Patch

Attachment 313097 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3942965

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 313106 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 313097 [details]
Patch

Attachment 313097 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3943033

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 313108 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 313097 [details]
Patch

Attachment 313097 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3943000

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 313109 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

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

[Build Bot]

Comment on attachment 313097 [details]
Patch

Attachment 313097 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3943002

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 313111 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Sam Weinig]

Created attachment 313118 [details]
Patch

[Sam Weinig]

Created attachment 313128 [details]
Patch

[Darin Adler]

Comment on attachment 313128 [details]
Patch

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

Would be nice to line up our terminology with the specification even closer. Can we use the term "responsible document" for example?

> Source/WebCore/ChangeLog:109
> [phone]  Sam Weinig  <[e-mail]>

Double change log.

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:5344
> +        AddToImplIncludes("Document.h");

Not sure I understand why this is required.

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:5352
> +        AddToImplIncludes("Document.h");

Ditto.

> Source/WebCore/dom/Document.cpp:2663
> +    return;

Should not add this. I assume it’s a leftover from refactoring.

> Source/WebCore/dom/Document.cpp:7257
> +void Document::clear()
> +{
> +    // Per https://html.spec.whatwg.org/multipage/obsolete.html#dom-document-clear, this method does nothing.
> +}
> +
> +void Document::captureEvents()
> +{
> +    // Per https://html.spec.whatwg.org/multipage/obsolete.html#dom-document-captureevents, this method does nothing.
> +}
> +
> +void Document::releaseEvents()
> +{
> +    // Per https://html.spec.whatwg.org/multipage/obsolete.html#dom-document-releaseevents, this method does nothing.
> +}

inline?

> Source/WebCore/dom/Document.h:601
>      WEBCORE_EXPORT void open(Document* ownerDocument = nullptr);

Maybe rename this at some point? Even possible at that time that we might want to rename openForBindings back to open.

> Source/WebCore/dom/Document.h:605
>      WEBCORE_EXPORT void close();

Maybe rename this at some point? Even possible at that time that we might want to rename closeForBindings back to close.

> Source/WebCore/dom/Document.h:1335
> +    WEBCORE_EXPORT const AtomicString& linkColorForBindings() const;
> +    WEBCORE_EXPORT void setLinkColorForBindings(const String&);

Do we really need the existing linkColor and setLinkColor? Can we rename those instead?

[Build Bot]

Comment on attachment 313128 [details]
Patch

Attachment 313128 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3944031

New failing tests:
js/dom/dom-attributes-on-mismatch-type.html
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/closing-the-input-stream/document.close-01.xhtml
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/opening-the-input-stream/document.open-01.xhtml
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/opening-the-input-stream/document.open-02.html
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/document-write/document.write-01.xhtml
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/document-writeln/document.writeln-01.xhtml

[Build Bot]

Created attachment 313144 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 313128 [details]
Patch

Attachment 313128 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3943999

New failing tests:
js/dom/dom-attributes-on-mismatch-type.html
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/closing-the-input-stream/document.close-01.xhtml
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/opening-the-input-stream/document.open-01.xhtml
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/opening-the-input-stream/document.open-02.html
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/document-write/document.write-01.xhtml
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/document-writeln/document.writeln-01.xhtml

[Build Bot]

Created attachment 313145 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

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

[Build Bot]

Comment on attachment 313128 [details]
Patch

Attachment 313128 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3944157

New failing tests:
js/dom/dom-attributes-on-mismatch-type.html
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/closing-the-input-stream/document.close-01.xhtml
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/opening-the-input-stream/document.open-01.xhtml
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/opening-the-input-stream/document.open-02.html
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/document-write/document.write-01.xhtml
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/document-writeln/document.writeln-01.xhtml

[Build Bot]

Created attachment 313146 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 313128 [details]
Patch

Attachment 313128 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3944071

New failing tests:
js/dom/dom-attributes-on-mismatch-type.html
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/closing-the-input-stream/document.close-01.xhtml
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/opening-the-input-stream/document.open-01.xhtml
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/opening-the-input-stream/document.open-02.html
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/document-write/document.write-01.xhtml
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/document-writeln/document.writeln-01.xhtml

[Build Bot]

Created attachment 313147 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Sam Weinig]

(In reply to Darin Adler from comment #30)
> Comment on attachment 313128 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=313128&action=review
> 
> Would be nice to line up our terminology with the specification even closer.
> Can we use the term "responsible document" for example?

I renamed CallerDocument to ResponsibleDocument and added a more clear comment.

> 
> > Source/WebCore/ChangeLog:109
> > [phone]  Sam Weinig  <[e-mail]>
> 
> Double change log.

Fixed.

> > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:5344
> > +        AddToImplIncludes("Document.h");
> 
> Not sure I understand why this is required.
> 
> > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:5352
> > +        AddToImplIncludes("Document.h");
> 
> Ditto.

They shouldn't be. Removed.

> 
> > Source/WebCore/dom/Document.cpp:2663
> > +    return;
> 
> Should not add this. I assume it’s a leftover from refactoring.

Yup. Removed.

> 
> > Source/WebCore/dom/Document.cpp:7257
> > +void Document::clear()
> > +{
> > +    // Per https://html.spec.whatwg.org/multipage/obsolete.html#dom-document-clear, this method does nothing.
> > +}
> > +
> > +void Document::captureEvents()
> > +{
> > +    // Per https://html.spec.whatwg.org/multipage/obsolete.html#dom-document-captureevents, this method does nothing.
> > +}
> > +
> > +void Document::releaseEvents()
> > +{
> > +    // Per https://html.spec.whatwg.org/multipage/obsolete.html#dom-document-releaseevents, this method does nothing.
> > +}
> 
> inline?

Inlined.

> 
> > Source/WebCore/dom/Document.h:601
> >      WEBCORE_EXPORT void open(Document* ownerDocument = nullptr);
> 
> Maybe rename this at some point? Even possible at that time that we might
> want to rename openForBindings back to open.
> 
> > Source/WebCore/dom/Document.h:605
> >      WEBCORE_EXPORT void close();
> 
> Maybe rename this at some point? Even possible at that time that we might
> want to rename closeForBindings back to close.

Added comments. Follow up.

> 
> > Source/WebCore/dom/Document.h:1335
> > +    WEBCORE_EXPORT const AtomicString& linkColorForBindings() const;
> > +    WEBCORE_EXPORT void setLinkColorForBindings(const String&);
> 
> Do we really need the existing linkColor and setLinkColor? Can we rename
> those instead?

I tried to think of an alternate name for the existing linkColor and came up blank. If I think of one I'll come back and fix it.

Thanks for the review!

[Sam Weinig]

Created attachment 313150 [details]
Patch

[Build Bot]

Comment on attachment 313150 [details]
Patch

Attachment 313150 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3944653

New failing tests:
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/opening-the-input-stream/document.open-02.html

[Build Bot]

Created attachment 313162 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 313150 [details]
Patch

Attachment 313150 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3944667

New failing tests:
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/opening-the-input-stream/document.open-02.html

[Build Bot]

Created attachment 313163 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 313150 [details]
Patch

Attachment 313150 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3944682

New failing tests:
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/opening-the-input-stream/document.open-02.html

[Build Bot]

Created attachment 313165 [details]
Archive of layout-test-results from ews113 for mac-elcapitan

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

[Build Bot]

Comment on attachment 313150 [details]
Patch

Attachment 313150 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3944695

New failing tests:
imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/opening-the-input-stream/document.open-02.html

[Build Bot]

Created attachment 313166 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Sam Weinig]

Committed r218437: <http://trac.webkit.org/changeset/218437>

[Frédéric Wang (:fredw)]

Committed r233503: <https://trac.webkit.org/changeset/233503>