1556358 - Custom elements: implement custom element reaction's formStateRestoreCallback

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[John Dai[:jdai]]

Spec:
https://html.spec.whatwg.org/multipage/custom-elements.html#custom-element-reactions

Comment 1

[Edgar Chen [:edgar]]

We need to figure out how to support form restoration or autocomplete for form-associated custom elements first.

Comment 2

[Adam Vandolder [:avandolder]]

Attachment: Bug 1556358 - Part 1: Add formStateRestore CE lifecycle callback. r?edgar

Comment 3

[Adam Vandolder [:avandolder]]

Attachment: Bug 1556358 - Part 2: Save and restore custom element form data. r?edgar

Depends on D174114

Comment 4

[clshortfuse]

Hi team.

Just to alert you that on Edge (Chromium based), there is a bug that related to the value being passed instead of the state:

https://bugs.chromium.org/p/chromium/issues/detail?id=1429585

Edge leaves live pages in a broken state, where as Chrome (stable) has not implemented yet.

Looking at a D174115 test, it treats state as value, which may pass, but note that state could be something like 'checked' or 'unchecked' which is to be parsed into a computed value (eg: checked). This near-exact example is given in the spec.

A test that ensures that null is passed for an unchecked radio-button from a page that called ElementInternals.setFormValue(null) could ensure we avoid some possible bugs.

Comment 5

[Adam Vandolder [:avandolder]]

I realized that the method I was using to test the patch is actually broken right now, and won't be possible until bug 1502814 gets fixed.

Comment 6

[Adam Vandolder [:avandolder]]

Attachment: Bug 1556358 - Allow FormData::ForEach to take a closure instead of a raw function pointer. r?edgar

Depends on D174114

Comment 7

[Edgar Chen [:edgar]]

Attachment: Bug 1556358 - Nullable union used as callback arguments needs to be actually declared; r?peterv

Comment 8

[Adam Vandolder [:avandolder]]

Attachment: Bug 1556358 - Part 3: Restore FACE state in SessionStore. r?edgar

Depends on D174115

Comment 9

[Phabricator Automation]

Comment on attachment 9329660 [details]
Bug 1556358 - Nullable union used as callback arguments needs to be actually declared; r?peterv

Revision D176127 was moved to bug 1836127. Setting attachment 9329660 [details] to obsolete.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2492572ea78a
Part 1: Add formStateRestore CE lifecycle callback. r=edgar

Comment 11

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/2492572ea78a

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a07e3e226569
Allow FormData::ForEach to take a closure instead of a raw function pointer. r=edgar
https://hg.mozilla.org/integration/autoland/rev/c41794eef66a
Part 2: Save and restore custom element form data. r=edgar
https://hg.mozilla.org/integration/autoland/rev/0ebda393786b
Part 3: Restore FACE state in SessionStore. r=edgar,farre

Comment 13

[Noemi Erli[:noemi_erli]]

Backed out 3 changesets (Bug 1556358) for causing bustages in ElementInternals.cpp CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&selectedTaskRun=KqTZqnymRgahnR7uEPg0aw.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=0ebda393786b0626290fd39412c47a8ba21e2171&searchStr=linux%2Cx64%2Cdebug%2Cbuild-linux64-base-toolchains%2Fdebug%2Cbb

Log: https://treeherder.mozilla.org/logviewer?job_id=419337358&repo=autoland&lineNumber=10831
https://treeherder.mozilla.org/logviewer?job_id=419347156&repo=autoland&lineNumber=2134

Backout: https://hg.mozilla.org/integration/autoland/rev/f9a60b1fb1fcbdf257a253cd78f89de6fe9b3b5e

Comment 14

[Adam Vandolder [:avandolder]]

The failure during test-verify appears to be a result of a pref changing in the background, unrelated to these patches; it should be fixed once https://bugzilla.mozilla.org/show_bug.cgi?id=1838233 lands.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/413225d91666
Allow FormData::ForEach to take a closure instead of a raw function pointer. r=edgar
https://hg.mozilla.org/integration/autoland/rev/c80f29a9cefc
Part 2: Save and restore custom element form data. r=edgar
https://hg.mozilla.org/integration/autoland/rev/f7e94ea82252
Part 3: Restore FACE state in SessionStore. r=edgar,farre

Comment 16

[Serban Stanca [:SerbanS]]

Backed out for causing Bb build bustages in ElementInternals.cpp.

• Backout link
• Push with failures
• Failure Log
• Failure line: /builds/worker/checkouts/gecko/dom/html/ElementInternals.cpp:473:10: error: no matching function for call to 'nsContentUtils::EnqueueLifecycleCallback(mozilla::dom::ElementCallbackType, RefPtr<mozilla::dom::HTMLElement>&, <brace-enclosed initializer list>)'

Comment 17

[Adam Vandolder [:avandolder]]

(In reply to Serban Stanca [:SerbanS] from comment #16)

Backed out for causing Bb build bustages in ElementInternals.cpp.

• Backout link
• Push with failures
• Failure Log
• Failure line: /builds/worker/checkouts/gecko/dom/html/ElementInternals.cpp:473:10: error: no matching function for call to 'nsContentUtils::EnqueueLifecycleCallback(mozilla::dom::ElementCallbackType, RefPtr<mozilla::dom::HTMLElement>&, <brace-enclosed initializer list>)'

Hmm. I'll admit I have no idea what is causing this failure. The changes from D174114 should have already landed, so that call should be fine. I guess I'll try landing the patches one at a time and see if that works instead.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/27a735f444d9
Allow FormData::ForEach to take a closure instead of a raw function pointer. r=edgar

Comment 19

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/27a735f444d9

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1680323680fe
Part 2: Save and restore custom element form data. r=edgar

Comment 21

[Norisz Fay [:noriszfay]]

Backed out for causing Bb bustages on ElementInternals.cpp

Backout link

Push with failures

Failure log

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/08f12e7fa5f0
Part 2: Save and restore custom element form data. r=edgar

Comment 23

[Edgar Chen [:edgar]]

(lets reopen this first and close once all part is stick)

Comment 24

[Edgar Chen [:edgar]]

(And I think we are no longer depend on bug 1502814 as tests now use same document to test)

Comment 25

[Natalia Csoregi [:nataliaCs]]

Backed out for causing bustage on ElementInternals.cpp

• backout: https://hg.mozilla.org/integration/autoland/rev/9b4582f72278af7055aa5c0c7656b2f2b313ff44
• push: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=cPh49cewSECqhrrWlCqAYw.0&revision=08f12e7fa5f04d9acf47a841046c192b3b6b9b71
• failure log: https://treeherder.mozilla.org/logviewer?job_id=419874849&repo=autoland&lineNumber=10469

[task 2023-06-19T17:40:25.512Z] 17:40:25     INFO -  In file included from Unified_cpp_dom_html0.cpp:11:
[task 2023-06-19T17:40:25.512Z] 17:40:25     INFO -  /builds/worker/checkouts/gecko/dom/html/ElementInternals.cpp: In member function 'void mozilla::dom::ElementInternals::RestoreFormValue(mozilla::dom::Nullable<mozilla::dom::OwningFileOrUSVStringOrFormData>&&, mozilla::dom::Nullable<mozilla::dom::OwningFileOrUSVStringOrFormData>&&)':
[task 2023-06-19T17:40:25.512Z] 17:40:25    ERROR -  /builds/worker/checkouts/gecko/dom/html/ElementInternals.cpp:473:10: error: no matching function for call to 'nsContentUtils::EnqueueLifecycleCallback(mozilla::dom::ElementCallbackType, RefPtr<mozilla::dom::HTMLElement>&, <brace-enclosed initializer list>)'
[task 2023-06-19T17:40:25.512Z] 17:40:25     INFO -           });
[task 2023-06-19T17:40:25.512Z] 17:40:25     INFO -            ^

Comment 26

[Adam Vandolder [:avandolder]]

(In reply to Adam Vandolder [:avandolder] from comment #17)

(In reply to Serban Stanca [:SerbanS] from comment #16)

Backed out for causing Bb build bustages in ElementInternals.cpp.

• Backout link
• Push with failures
• Failure Log
• Failure line: /builds/worker/checkouts/gecko/dom/html/ElementInternals.cpp:473:10: error: no matching function for call to 'nsContentUtils::EnqueueLifecycleCallback(mozilla::dom::ElementCallbackType, RefPtr<mozilla::dom::HTMLElement>&, <brace-enclosed initializer list>)'

Hmm. I'll admit I have no idea what is causing this failure. The changes from D174114 should have already landed, so that call should be fine. I guess I'll try landing the patches one at a time and see if that works instead.

So I think the cause is that, for whatever reason, this one specific linux64-debug build uses --std=gnu++17, which might not properly support aggregate initialization with designated initializers as they are a C++20 feature. At least now I know the problem wasn't how I was trying to land the stack.

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9fb58fa7b8b6
Part 2: Save and restore custom element form data. r=edgar

Comment 28

[Iulian Moraru]

Backed out for causing wpt failures on ElementInternals-setFormValue.html.

• Push where we found this

• Push with failures

• Failure log

• Backout link

[task 2023-06-20T05:04:04.417Z] 05:04:04     INFO - TEST-START | /custom-elements/form-associated/ElementInternals-setFormValue.html
[task 2023-06-20T05:04:04.433Z] 05:04:04     INFO - Closing window 16dcadac-9476-4255-ac1f-978c1f3703c4
[task 2023-06-20T05:04:06.977Z] 05:04:06     INFO - PID 7319 | -----------------------------------------------------
[task 2023-06-20T05:04:06.977Z] 05:04:06     INFO - PID 7319 | Suppressions used:
[task 2023-06-20T05:04:06.977Z] 05:04:06     INFO - PID 7319 |   count      bytes template
[task 2023-06-20T05:04:06.978Z] 05:04:06     INFO - PID 7319 |       2        288 libfontconfig.so
[task 2023-06-20T05:04:06.978Z] 05:04:06     INFO - PID 7319 | -----------------------------------------------------
[task 2023-06-20T05:04:07.579Z] 05:04:07     INFO - 
[task 2023-06-20T05:04:07.579Z] 05:04:07     INFO - TEST-PASS | /custom-elements/form-associated/ElementInternals-setFormValue.html | Single value - name is missing 
[task 2023-06-20T05:04:07.579Z] 05:04:07     INFO - TEST-PASS | /custom-elements/form-associated/ElementInternals-setFormValue.html | Single value - empty name exists 
[task 2023-06-20T05:04:07.579Z] 05:04:07     INFO - TEST-UNEXPECTED-FAIL | /custom-elements/form-associated/ElementInternals-setFormValue.html | Single value - Non-empty name exists - assert_equals: expected "?name-pd1=value-pd1&name-usv=abc%EF%BF%BD%EF%BF%BDdef&name-file=test_file.txt" but got "?name-pd1=value-pd1&name-ce1=value-ce1&name-usv=abc%EF%BF%BD%EF%BF%BDdef&name-file=test_file.txt"
[task 2023-06-20T05:04:07.579Z] 05:04:07     INFO - @http://web-platform.test:8000/custom-elements/form-associated/ElementInternals-setFormValue.html:153:18
[task 2023-06-20T05:04:07.579Z] 05:04:07     INFO - 
[task 2023-06-20T05:04:07.579Z] 05:04:07     INFO - TEST-UNEXPECTED-FAIL | /custom-elements/form-associated/ElementInternals-setFormValue.html | Null value should submit nothing - assert_equals: expected "?name-pd1=value-pd1" but got "?name-pd1=value-pd1&name-ce2=abc%EF%BF%BD%EF%BF%BDdef"
[task 2023-06-20T05:04:07.579Z] 05:04:07     INFO - @http://web-platform.test:8000/custom-elements/form-associated/ElementInternals-setFormValue.html:166:18
[task 2023-06-20T05:04:07.595Z] 05:04:07     INFO - ...................................................
[task 2023-06-20T05:04:07.596Z] 05:04:07     INFO - TEST-OK | /custom-elements/form-associated/ElementInternals-setFormValue.html | took 3168ms

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3561a683c068
Part 2: Save and restore custom element form data. r=edgar

Comment 30

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/3561a683c068

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9c5d63981831
Part 3: Restore FACE state in SessionStore. r=edgar,farre

Comment 32

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/9c5d63981831

Comment 33

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Adam, is there anything planned to be done in this bug? If not, please remove the leave-open keyword and close this. Thank you!

Comment 34

[Dianna Smith [:diannaS]]

:avandolder is this something you want to nominate for fx116 relnotes?

Comment 35

[Adam Vandolder [:avandolder]]

Release Note Request (optional, but appreciated)
[Why is this notable]: Adds support for a new reaction callback on custom element definitions, formStateRestoreCallback.
[Affects Firefox for Android]: n/a
[Suggested wording]: Usage of the formStateRestoreCallback custom element reaction callback is now supported, allowing developers to add support for browser's attempting to restore prior user state. Note that 'autocomplete' for custom elements remains unsupported.
[Links (documentation, blog post, etc)]: Specification is at https://html.spec.whatwg.org/multipage/custom-elements.html#custom-element-reactions, an explanatory post from the Chrome devs is at https://web.dev/more-capable-form-controls/

Comment 36

[Dianna Smith [:diannaS]]

Added to 116 release notes, currently in beta https://www.mozilla.org/en-US/firefox/116.0beta/releasenotes/