XML Parser should invoke reactions when creating/inserting new custom elements

• Component: DOM
• Status: RESOLVED FIXED

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

From https://html.spec.whatwg.org/multipage/parsing.html#create-an-element-for-the-token:

6.3 Push a new element queue onto the custom element reactions stack.
9.1 Let queue be the result of popping the current element queue from the custom element reactions stack. (This will be the same element queue as was pushed above.)
9.2 Invoke custom element reactions in queue.

This should also be done for the XML parser per https://html.spec.whatwg.org/multipage/xhtml.html#parsing-xhtml-documents

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

Created attachment 346146 [details]
WIP Patch

[EWS Watchlist]

Attachment 346146 [details] did not pass style-queue:


ERROR: Source/WebCore/html/parser/HTMLConstructionSite.h:107:  The parameter name "element" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

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

Comment 0 was during element creation (and I guess essentially for attributeChanged reaction).

The same thing should be done when inserting the node (and I guess essentially for connectedCallback reaction):

If it is possible to insert element at the adjusted insertion location, then:
    Push a new element queue onto the custom element reactions stack.
    Insert element at the adjusted insertion location.
    Pop the element queue from the custom element reactions stack, and invoke custom element reactions in that queue.

(https://html.spec.whatwg.org/#insert-a-foreign-element)

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

Created attachment 346152 [details]
WIP Patch

[EWS Watchlist]

Comment on attachment 346152 [details]
WIP Patch

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

New failing tests:
fast/css/insert-rule-overflow-rule-data.html
fast/text/large-text-composed-char.html
imported/blink/fast/text/wide-preformatted.html
js/function-apply-many-args.html
http/tests/misc/webtiming-slow-load.php
fast/css/css-selector-deeply-nested.html

[EWS Watchlist]

Created attachment 346155 [details]
Archive of layout-test-results from ews101 for mac-sierra

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

[EWS Watchlist]

Comment on attachment 346152 [details]
WIP Patch

Attachment 346152 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8709296

New failing tests:
fast/css/insert-rule-overflow-rule-data.html
fast/text/large-text-composed-char.html
imported/blink/fast/text/wide-preformatted.html
js/function-apply-many-args.html
http/tests/misc/webtiming-slow-load.php
fast/css/css-selector-deeply-nested.html

[EWS Watchlist]

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

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

[EWS Watchlist]

Comment on attachment 346152 [details]
WIP Patch

Attachment 346152 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/8709346

New failing tests:
fast/css/insert-rule-overflow-rule-data.html
js/function-apply-many-args.html
platform/ios/ios/storage/domstorage/5mb-quota.html
imported/blink/fast/text/wide-preformatted.html
http/tests/misc/webtiming-slow-load.php
fast/css/css-selector-deeply-nested.html

[EWS Watchlist]

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

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

[EWS Watchlist]

Comment on attachment 346152 [details]
WIP Patch

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

New failing tests:
fast/css/insert-rule-overflow-rule-data.html
fast/text/large-text-composed-char.html
imported/blink/fast/text/wide-preformatted.html
js/function-apply-many-args.html
http/tests/misc/webtiming-slow-load.php
fast/css/css-selector-deeply-nested.html

[EWS Watchlist]

Created attachment 346163 [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 346152 [details]
WIP Patch

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

New failing tests:
fast/css/insert-rule-overflow-rule-data.html
fast/text/large-text-composed-char.html
imported/blink/fast/text/wide-preformatted.html
js/function-apply-many-args.html
http/tests/misc/webtiming-slow-load.php
fast/css/css-selector-deeply-nested.html

[EWS Watchlist]

Created attachment 346166 [details]
Archive of layout-test-results from ews117 for mac-sierra

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

[EWS Watchlist]

Comment on attachment 346152 [details]
WIP Patch

Attachment 346152 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8712411

New failing tests:
js/function-apply-many-args.html
fast/css/css-selector-deeply-nested.html
fast/css/insert-rule-overflow-rule-data.html
imported/blink/fast/text/wide-preformatted.html

[EWS Watchlist]

Created attachment 346189 [details]
Archive of layout-test-results from ews201 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews201  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Radar WebKit Bug Importer]

<rdar://problem/42843008>

[Ryosuke Niwa]

Comment on attachment 346152 [details]
WIP Patch

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

In general, implementing things exactly the way spec is not warranted especially in the code that affects perf.
We just need to match the end behavior.

> Source/WebCore/html/parser/HTMLConstructionSite.cpp:128
> +    if (UNLIKELY(task.mayRunConnectedCallback))
> +        customElementReactionStack.emplace();
>      insert(task);
> +    customElementReactionStack.reset();

Adding this much code in this hot function is probably not acceptable in terms of perf.

[Ryosuke Niwa]

Comment on attachment 346152 [details]
WIP Patch

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

> Source/WebCore/html/parser/HTMLDocumentParser.cpp:226
> +        if (willExecuteScript) {

This check is unnecessary. If we're making a fallback element, then creating a custom element reaction stack has no observable effect.

[Ryosuke Niwa]

I posted a patch on https://bugs.webkit.org/show_bug.cgi?id=188336 to solve some of the issues this patch tries to address.

[Ryosuke Niwa]

Created attachment 461337 [details]
WIP

[Ryosuke Niwa]

Pull request: https://github.com/WebKit/WebKit/pull/2959

[EWS]

Committed 253122@main (6b27b1ffda33): <https://commits.webkit.org/253122@main>

Reviewed commits have been landed. Closing PR #2959 and removing active labels.