Skip to content

Add optional submitter parameter to FormData constructor#9188

Merged
webkit-early-warning-system merged 1 commit intoWebKit:mainfrom
jenseng:formdata-constructor-submitter
Jan 30, 2023
Merged

Add optional submitter parameter to FormData constructor#9188
webkit-early-warning-system merged 1 commit intoWebKit:mainfrom
jenseng:formdata-constructor-submitter

Conversation

@jenseng
Copy link
Copy Markdown
Contributor

@jenseng jenseng commented Jan 26, 2023 •
edited by webkit-early-warning-system
Loading

136a23f

Add optional submitter parameter to FormData constructor
https://bugs.webkit.org/show_bug.cgi?id=251220

Reviewed by Chris Dumez

Implement the new submitter parameter to the FormData constructor
Spec PR: whatwg/xhr#366
WPT PR: web-platform-tests/wpt#37895

Test: imported/w3c/web-platform-tests/xhr/formdata/constructor-submitter.html

* LayoutTests/imported/w3c/web-platform-tests/xhr/formdata/constructor-submitter-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/xhr/formdata/constructor-submitter.html: Added.
* Source/WebCore/html/DOMFormData.cpp:
(WebCore::DOMFormData::create): add submitter support
* Source/WebCore/html/DOMFormData.h: update signature
* Source/WebCore/html/DOMFormData.idl: update interface
* Source/WebCore/html/HTMLFormElement.cpp:
(WebCore::HTMLFormElement::requestSubmit): improve error messages#

Canonical link: https://commits.webkit.org/259558@main

86174d6

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ❌ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ❌ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 watch-sim
✅ 🛠 🧪 unsafe-merge

@jenseng jenseng requested review from cdumez and rniwa as code owners January 26, 2023 21:49
@webkit-early-warning-system
Copy link
Copy Markdown
Collaborator

webkit-early-warning-system commented Jan 26, 2023 •
edited
Loading

EWS run on previous version of this PR (hash 54e479b)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ✅ 🧪 api-mac ❌ 🧪 gtk-wk2
✅ 🧪 api-ios ❌ 🧪 mac-wk1 ❌ 🧪 api-gtk
✅ 🛠 tv ❌ 🧪 mac-wk2
✅ 🛠 tv-sim ❌ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 watch-sim

@jenseng
Copy link
Copy Markdown
Contributor Author

jenseng commented Jan 26, 2023

The tests included are the same as in web-platform-tests/wpt#37895, I just dropped them under LayoutTests/imported/w3c/web-platform-tests/ ... let me know if there's a different approach I should take

cdumez
cdumez requested changes Jan 26, 2023
View reviewed changes
@cdumez
Copy link
Copy Markdown
Contributor

cdumez commented Jan 26, 2023

The tests included are the same as in web-platform-tests/wpt#37895, I just dropped them under LayoutTests/imported/w3c/web-platform-tests/ ... let me know if there's a different approach I should take

Right approach, just missing the expected results (*-expected.txt) as part of your patch, as I commented above.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 27, 2023
@jenseng jenseng force-pushed the formdata-constructor-submitter branch from 54e479b to 86174d6 Compare January 27, 2023 15:53
@webkit-early-warning-system
Copy link
Copy Markdown
Collaborator

webkit-early-warning-system commented Jan 27, 2023 •
edited
Loading

EWS run on current version of this PR (hash 86174d6)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ❌ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ❌ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 watch-sim
✅ 🛠 🧪 unsafe-merge

@jenseng
Copy link
Copy Markdown
Contributor Author

jenseng commented Jan 27, 2023

Addressed PR feedback, let me know if there's anything else this needs 🙏

cdumez
cdumez approved these changes Jan 27, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@cdumez cdumez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cdumez
Copy link
Copy Markdown
Contributor

cdumez commented Jan 27, 2023 •
edited
Loading

Addressed PR feedback, let me know if there's anything else this needs 🙏

Looks good, thanks!
I'll merge once the EWS bots confirm there are no regressions.

@jenseng jenseng mentioned this pull request Jan 28, 2023
Merged
@cdumez cdumez added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed merging-blocked Applied to prevent a change from being merged labels Jan 30, 2023
@jenseng @cdumez
Add optional submitter parameter to FormData constructor
136a23f 
https://bugs.webkit.org/show_bug.cgi?id=251220

Reviewed by Chris Dumez

Implement the new submitter parameter to the FormData constructor
Spec PR: whatwg/xhr#366
WPT PR: web-platform-tests/wpt#37895

Test: imported/w3c/web-platform-tests/xhr/formdata/constructor-submitter.html

* LayoutTests/imported/w3c/web-platform-tests/xhr/formdata/constructor-submitter-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/xhr/formdata/constructor-submitter.html: Added.
* Source/WebCore/html/DOMFormData.cpp:
(WebCore::DOMFormData::create): add submitter support
* Source/WebCore/html/DOMFormData.h: update signature
* Source/WebCore/html/DOMFormData.idl: update interface
* Source/WebCore/html/HTMLFormElement.cpp:
(WebCore::HTMLFormElement::requestSubmit): improve error messages#

Canonical link: https://commits.webkit.org/259558@main
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

webkit-commit-queue commented Jan 30, 2023

Committed 259558@main (136a23f): https://commits.webkit.org/259558@main

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

@webkit-early-warning-system webkit-early-warning-system merged commit 136a23f into WebKit:main Jan 30, 2023
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jan 30, 2023
@cdumez
Copy link
Copy Markdown
Contributor

cdumez commented Jan 30, 2023

Congratulations on your first WebKit PR 👍🏻

@jenseng
Copy link
Copy Markdown
Contributor Author

jenseng commented Jan 30, 2023

Awesome, thanks @cdumez!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cdumez cdumez cdumez approved these changes

@rniwa rniwa Awaiting requested review from rniwa rniwa is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jenseng @webkit-early-warning-system @cdumez @webkit-commit-queue @webkit-ews-buildbot