Bug 188190

Summary:

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

Product:

WebKit

Reporter:

Frédéric Wang (:fredw) <fred.wang>

Component:

DOM

Assignee:

Ryosuke Niwa <rniwa>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, esprehn+autocc, ews-watchlist, gyuyoung.kim, kangil.han, rniwa, rwlbuis, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

243441

Bug Blocks:

154907

Attachments:

Description	Flags
WIP Patch	none
WIP Patch	ews-watchlist: commit-queue-
Archive of layout-test-results from ews101 for mac-sierra	none
Archive of layout-test-results from ews104 for mac-sierra-wk2	none
Archive of layout-test-results from ews126 for ios-simulator-wk2	none
Archive of layout-test-results from ews115 for mac-sierra	none
Archive of layout-test-results from ews117 for mac-sierra	none
Archive of layout-test-results from ews201 for win-future	none
WIP	ews-feeder: commit-queue-

Frédéric Wang (:fredw)

Reported 2018-07-31 00:40:09 PDT

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

Attachments
WIP Patch (10.18 KB, patch) 2018-07-31 00:44 PDT, Frédéric Wang (:fredw)	no flags	Details Formatted Diff Diff
WIP Patch (13.46 KB, patch) 2018-07-31 04:21 PDT, Frédéric Wang (:fredw)	ews-watchlist: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews101 for mac-sierra (2.72 MB, application/zip) 2018-07-31 05:30 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews104 for mac-sierra-wk2 (3.17 MB, application/zip) 2018-07-31 05:37 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (2.65 MB, application/zip) 2018-07-31 06:18 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews115 for mac-sierra (3.49 MB, application/zip) 2018-07-31 06:25 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews117 for mac-sierra (3.45 MB, application/zip) 2018-07-31 08:05 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews201 for win-future (12.90 MB, application/zip) 2018-07-31 12:24 PDT, EWS Watchlist	no flags	Details
WIP (5.20 KB, patch) 2022-08-01 11:41 PDT, Ryosuke Niwa	ews-feeder: commit-queue-	Details Formatted Diff Diff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.

Frédéric Wang (:fredw)

Comment 1 2018-07-31 00:44:43 PDT

Created attachment 346146 [details] WIP Patch

EWS Watchlist

Comment 2 2018-07-31 00:46:58 PDT

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 3 2018-07-31 03:45:55 PDT

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)

Comment 4 2018-07-31 04:21:17 PDT

Created attachment 346152 [details] WIP Patch

EWS Watchlist

Comment 5 2018-07-31 05:30:39 PDT

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

Comment 6 2018-07-31 05:30:41 PDT

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 7 2018-07-31 05:37:10 PDT

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

Comment 8 2018-07-31 05:37:12 PDT

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 9 2018-07-31 06:18:14 PDT

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

Comment 10 2018-07-31 06:18:15 PDT

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 11 2018-07-31 06:25:52 PDT

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

Comment 12 2018-07-31 06:25:54 PDT

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 13 2018-07-31 08:05:19 PDT

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

Comment 14 2018-07-31 08:05:21 PDT

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 15 2018-07-31 12:24:01 PDT

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

Comment 16 2018-07-31 12:24:13 PDT

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

Comment 17 2018-08-01 22:41:56 PDT

<rdar://problem/42843008>

Ryosuke Niwa

Comment 18 2018-08-05 14:12:43 PDT

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 19 2018-08-05 18:39:34 PDT

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

Comment 20 2018-08-05 19:09:12 PDT

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

Comment 21 2022-08-01 11:41:12 PDT

Created attachment 461337 [details] WIP

Ryosuke Niwa

Comment 22 2022-08-02 13:19:07 PDT

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

EWS

Comment 23 2022-08-04 11:34:49 PDT

Committed 253122@main (6b27b1ffda33): <https://commits.webkit.org/253122@main> Reviewed commits have been landed. Closing PR #2959 and removing active labels.

Note You need to log in before you can comment on or make changes to this bug.