Closed Bug 1556358 Opened 5 years ago Closed 1 year ago

Custom elements: implement custom element reaction's formStateRestoreCallback

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
116 Branch
Tracking Flags:
Tracking Status
relnote-firefox --- 116+
firefox116 --- fixed

People

(Reporter: jdai, Assigned: avandolder)

References

(Blocks 2 open bugs,
URL
)

Details

Attachments

(4 files, 1 obsolete file)

Bug 1556358 - Part 1: Add formStateRestore CE lifecycle callback. r?edgar
48 bytes, text/x-phabricator-request
Details | Review
Bug 1556358 - Part 2: Save and restore custom element form data. r?edgar
48 bytes, text/x-phabricator-request
Details | Review
Bug 1556358 - Allow FormData::ForEach to take a closure instead of a raw function pointer. r?edgar
48 bytes, text/x-phabricator-request
Details | Review
Bug 1556358 - Part 3: Restore FACE state in SessionStore. r?edgar
48 bytes, text/x-phabricator-request
Details | Review
Assignee: jdai → nobody
Status: ASSIGNED → NEW

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

Priority: P2 → P3
Severity: normal normal → S3 S3
Assignee: nobody → avandolder
Status: NEW → ASSIGNED

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.

No longer depends on: 1734841
See Also: → 1734841
Depends on: 1502814

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 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.

Attachment #9329660 - Attachment is obsolete: true
Pushed by avandolder@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2492572ea78a
Part 1: Add formStateRestore CE lifecycle callback. r=edgar

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

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
status-firefox116: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 116 Branch
Pushed by avandolder@mozilla.com:
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
Regressions: 1838538

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.

Flags: needinfo?(avandolder)
Pushed by avandolder@mozilla.com:
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

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>)'
Flags: needinfo?(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.

Flags: needinfo?(avandolder)
Pushed by avandolder@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/27a735f444d9
Allow FormData::ForEach to take a closure instead of a raw function pointer. r=edgar
Pushed by avandolder@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1680323680fe
Part 2: Save and restore custom element form data. r=edgar

Backed out for causing Bb bustages on ElementInternals.cpp

Backout link

Push with failures

Failure log

Flags: needinfo?(avandolder)
Pushed by avandolder@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/08f12e7fa5f0
Part 2: Save and restore custom element form data. r=edgar

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

Status: RESOLVED → REOPENED
Keywords: leave-open
Resolution: FIXED → ---

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

No longer depends on: 1502814

Backed out for causing bustage on ElementInternals.cpp

[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 -            ^

(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.

Flags: needinfo?(avandolder)
Pushed by avandolder@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9fb58fa7b8b6
Part 2: Save and restore custom element form data. r=edgar

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

[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
Flags: needinfo?(avandolder)
Pushed by avandolder@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3561a683c068
Part 2: Save and restore custom element form data. r=edgar
Flags: needinfo?(avandolder)
Pushed by avandolder@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9c5d63981831
Part 3: Restore FACE state in SessionStore. r=edgar,farre

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

Flags: needinfo?(avandolder)
Status: REOPENED → RESOLVED
Closed: 1 year ago → 1 year ago
Flags: needinfo?(avandolder)
Keywords: leave-open
Resolution: --- → FIXED

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

status-firefox116: affected → fixed
Flags: needinfo?(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/

relnote-firefox: --- → ?
Flags: needinfo?(avandolder)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: