Closed Bug 1319342 Opened 3 years ago Closed 3 years ago

Cloning a node runs concept-create-element steps with synchronous-custom-elements-flag unset which enqueues an upgrade reaction

Categories

(Core :: DOM: Core & HTML, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: jdai, Assigned: jdai)

References

Details

(Whiteboard: dom-ce-m2)

Attachments

(1 file, 7 obsolete files)

+++ This bug was initially created as a follow-up of Bug #1309184 comment #9 +++

create-element can be synchronous or asynchronous [1]. For asynchronous case (e.g. clone a node [2]), we need to enqueue an upgrade reaction just like what we did for "definition comes after" case. Will you handle asynchronous case in this bug? If not, please file a follow-up bug for that.

[1] See "synchronous custom elements flag" in https://dom.spec.whatwg.org/#concept-create-element
[2] See step 2.1 of https://dom.spec.whatwg.org/#concept-node-clone
Whiteboard: dom-ce-m2
Summary: Asynchronous upgrade reaction for custom element reactions → Asynchronous upgrade when creating an element
Summary: Asynchronous upgrade when creating an element → Cloning a node runs concept-create-element steps with synchronous-custom-elements-flag unset which enqueues an upgrade reaction
Depends on: 1299363, 1309184
Attached patch patch, v1 (obsolete) — Splinter Review
Per spec, clone node will create an element and synchronous custom elements flag  be unset. It means that we need to enqueue a custom element upgrade reaction in our clone code for both autonomous custom elements and build-in custom elements. 

- Revise clone implementation for custom element v1.
- Comment out a v0 test which is test_custom_element_clone_callbacks.html. 


Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=009f948ca031780e9fa9852f543a6dcaf53f4d93&filter-tier=1&group_state=expanded
Flags: needinfo?(bugs)
Flags: needinfo?(bugs)
Attachment #8905052 - Flags: review?(bugs)
Comment on attachment 8905052 [details] [diff] [review]
patch, v1

Could you put the code inside the custom element pref check.

Couldn't we just remove test_custom_element_clone_callbacks.html and then line
[test_custom_element_clone_callbacks.html] from the .ini.
Attachment #8905052 - Flags: review?(bugs) → review+
Priority: -- → P2
Keywords: checkin-needed
Keywords: checkin-needed
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff9c67df213a
Clone a node should enqueue an upgrade reaction. r=smaug
Keywords: checkin-needed
Backed out for failing wpt /dom/nodes/Node-cloneNode.html:

https://hg.mozilla.org/integration/mozilla-inbound/rev/44c358447666fd00d38fe9b72724d194b874e682

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=ff9c67df213ac4facfd517652cf29f156ea409dc&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=133400200&repo=mozilla-inbound
12:46:44     INFO - TEST-PASS | /dom/nodes/Node-cloneNode.html | createElement(datalist) 
12:46:44     INFO - TEST-UNEXPECTED-FAIL | /dom/nodes/Node-cloneNode.html | createElement(dialog) - assert_true: HTMLDialogElement is not supported expected true got false
12:46:44     INFO - create_element_and_check/<@http://web-platform.test:8000/dom/nodes/Node-cloneNode.html:45:5
12:46:44     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1485:20
12:46:44     INFO - test@http://web-platform.test:8000/resources/testharness.js:511:9
12:46:44     INFO - create_element_and_check@http://web-platform.test:8000/dom/nodes/Node-cloneNode.html:44:3
12:46:44     INFO - @http://web-platform.test:8000/dom/nodes/Node-cloneNode.html:80:1
12:46:44     INFO - 

Push with more failing tests: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=075e7eef53f45a57e6363b19decafa1219578288&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Flags: needinfo?(jdai)
Attached patch patch, v4 (obsolete) — Splinter Review
Fix WPT failures.
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d845a55886abbae73cab589d240dde229c3d1af8&group_state=expanded&filter-tier=1
Attachment #8912202 - Attachment is obsolete: true
Flags: needinfo?(jdai)
Attachment #8912530 - Flags: review+
Attached patch patch, v5 (obsolete) — Splinter Review
Fixed WPT failures on windows and mac os.
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=adf6d14129dffa931dea025c6204998eee921104&filter-tier=1&group_state=expanded
Attachment #8912530 - Attachment is obsolete: true
Attachment #8912621 - Flags: review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d3ebcf7d4c31
Clone a node should enqueue an upgrade reaction. r=smaug
Keywords: checkin-needed
Backed out for failing web-platform-test /dom/nodes/Node-cloneNode.html:

https://hg.mozilla.org/integration/mozilla-inbound/rev/43b3df38a4068f7d5f0e281fdd30bf76a59d650c

Push which ran failing tests: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=be61a3dc120a6ebdbc213525fea45eedfded57ef&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=133615288&repo=mozilla-inbound

 TEST-UNEXPECTED-FAIL | /dom/nodes/Node-cloneNode.html | createElement(dialog) - assert_true: HTMLDialogElement is not supported expected true got false
TEST-UNEXPECTED-FAIL | /dom/nodes/Node-cloneNode.html | implementation.createDocument - assert_equals: origin value expected (string) "null" but got (undefined) undefined
Flags: needinfo?(jdai)
Attached patch patch, v6 (obsolete) — Splinter Review
Rebase and fix wpt failures.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8c0da37236edd5e41ab10c74c5a755d1fd71a731&group_state=expanded
Attachment #8912621 - Attachment is obsolete: true
Flags: needinfo?(jdai)
Attachment #8914201 - Flags: review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/29a6664e8f73
Clone a node should enqueue an upgrade reaction. r=smaug
Keywords: checkin-needed
Backout by kwierso@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/e35c81ca2b65
Backed out changeset 29a6664e8f73 for wpt failures in Node-cloneNode.html a=backout
Keywords: checkin-needed
It would be nice to only change clone node relative wpt .ini files. I'll update a new patch to reflect this change.
Keywords: checkin-needed
Attached patch patch, v8Splinter Review
Revert dom/nodes/Node-cloneNode.html.ini change.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3c8128613f89d2f3479bbf11f9566dd50514697&filter-tier=1&group_state=expanded
Attachment #8914634 - Attachment is obsolete: true
Attachment #8914646 - Flags: review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3bc98b219437
Clone a node should enqueue an upgrade reaction. r=smaug
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/3bc98b219437
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.