Form constraint validation #25447

teapotd · 2020-01-06T19:47:00Z

It's almost done, there are few things remaining:

~~Range underflow, range overflow and step mismatch implementation require #25405~~
~~There are some test failures due to missing DOM parts (#25003)~~
~~pattern attribute uses JS regexp syntax. Currently I used regex crate, but it's probably incompatible. Should we use SpiderMonkey's regexp via jsapi?~~
Currently validation errors are reported using println!. Are there any better options?
~~"While the user interface is representing input that the user agent cannot convert to punycode, the control is suffering from bad input."~~

r? @jdm

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #11444
There are tests for these changes

highfive · 2020-01-06T19:47:10Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/dom/webidls/HTMLButtonElement.webidl, components/script/dom/htmlselectelement.rs, components/script/dom/webidls/HTMLOutputElement.webidl, components/script/textinput.rs, components/script/dom/webidls/HTMLFormElement.webidl and 15 more
@KiChjang: components/script/dom/webidls/HTMLButtonElement.webidl, components/script/dom/htmlselectelement.rs, components/script/dom/webidls/HTMLOutputElement.webidl, components/script/textinput.rs, components/script/dom/webidls/HTMLFormElement.webidl and 15 more

bors-servo · 2020-01-06T20:14:57Z

☔ The latest upstream changes (presumably #25424) made this pull request unmergeable. Please resolve the merge conflicts.

servo-wpt-sync · 2020-01-08T11:01:58Z

Error syncing changes upstream. Logs saved in error-snapshot-1578481317370.

bors-servo · 2020-01-09T04:35:56Z

☔ The latest upstream changes (presumably #25462) made this pull request unmergeable. Please resolve the merge conflicts.

servo-wpt-sync · 2020-01-10T16:11:52Z

Opened new PR for upstreamable changes.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21135.

servo-wpt-sync · 2020-01-11T11:45:34Z

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21135.

servo-wpt-sync · 2020-01-11T19:27:46Z

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21135.

servo-wpt-sync · 2020-01-12T11:33:50Z

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21135.

servo-wpt-sync · 2020-01-13T17:58:41Z

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21135.

teapotd · 2020-01-13T18:39:06Z

I think it's ready for a review. Some notes:

~~A few rangeUnderflow and rangeOverflow tests don't pass due to #25493, should be fixed by #25494~~
~~I hardcoded JSREG_UNICODE flag for now, because there are some issues with mozjs upgrade (servo/mozjs#224, servo/rust-mozjs#493)~~
The stepMismatch verification is problematic due to rounding errors, see whatwg/html#5207. Firefox parses values as decimals to avoid those issues. Current WPT tests also seem to expect exact precision. I think we should go with decimals as well, but it would require adding a crate and changes to value-as-number conversion. I went with epsilon comparison for now.
Test for nameless radio button in html/semantics/forms/constraints/form-validation-validity-valueMissing.html is inconsistent with spec, see whatwg/html#5202 (comment).

@jdm Let me know what you think!

servo-wpt-sync · 2020-01-13T18:49:13Z

PR title changed; changed existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21135.

jdm

Great work! I found this code to be quite readable. Thanks for adding all of the useful specification links!

components/script/dom/validation.rs

jdm · 2020-01-23T11:28:35Z

components/script/dom/htmlinputelement.rs

+            JS_ClearPendingException(*cx);
+        }
+
+        !ok || !rval.is_null()


Don't we want ok && !rval.is_null()? Am I misunderstanding what JS_ExecuteRegExpNoStatics returns?

It returns false on error. If pattern doesn't match it returns true and set rval to null:

https://github.com/servo/mozjs/blob/b2f83932fe9d361face14efd03f2465b9262e687/mozjs/js/src/builtin/RegExp.cpp#L167-L177

I guess errors should be treated the same way as invalid patterns, i.e. element shouldn't suffer from pattern mismatch.

I changed it to return Result<bool, ()> instead of bool to make it less confusing and errors are ignored in suffers_from_pattern_mismatch.


        Auto merge of #237 - teapotd:regexp-bindings, r=jdm

Add RegExp bindings I finally got around to finishing [form validation](servo/servo#25447) in Servo and it looks like RegExp bindings got lost after SpiderMonkey update. This PR adds `js/RegExp.h` to `jsglue.hpp`.


        Auto merge of #237 - teapotd:regexp-bindings, r=jdm

Add RegExp bindings I finally got around to finishing [form validation](servo/servo#25447) in Servo and it looks like RegExp bindings got lost after SpiderMonkey update. This PR adds `js/RegExp.h` to `jsglue.hpp`.

bors-servo · 2020-04-01T22:43:22Z

💔 Test failed - status-taskcluster

jdm · 2020-04-02T02:19:35Z

@bors-servo retry

#25513 #24622

bors-servo · 2020-04-02T02:54:49Z

⌛ Testing commit 8b21e06 with merge fbd1559...


        Auto merge of #25447 - teapotd:form-validation, r=jdm

@jdm

Form constraint validation It's almost done, there are few things remaining: - ~Range underflow, range overflow and step mismatch implementation require #25405~ - ~There are some test failures due to missing DOM parts (#25003)~ - ~`pattern` attribute uses JS regexp syntax. Currently I used regex crate, but it's probably incompatible. Should we use SpiderMonkey's regexp via jsapi?~ - Currently validation errors are reported using `println!`. Are there any better options? - ~["While the user interface is representing input that the user agent cannot convert to punycode, the control is suffering from bad input."](https://html.spec.whatwg.org/multipage/#e-mail-state-(type%3Demail)%3Asuffering-from-bad-input)~ r? @jdm --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #11444 - [x] There are tests for these changes

bors-servo · 2020-04-02T03:38:10Z

💔 Test failed - status-taskcluster

jdm · 2020-04-02T03:48:59Z

@bors-servo retry

bors-servo · 2020-04-02T03:54:50Z

🔒 Merge conflict

bors-servo · 2020-04-02T03:57:25Z

☔ The latest upstream changes (presumably #26086) made this pull request unmergeable. Please resolve the merge conflicts.


        Form constraints validation

servo-wpt-sync · 2020-04-02T09:30:40Z

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21135.

jdm · 2020-04-02T11:26:49Z

@bors-servo r+

bors-servo · 2020-04-02T11:26:50Z

📌 Commit 779552e has been approved by jdm

bors-servo · 2020-04-02T12:07:32Z

⌛ Testing commit 779552e with merge a651bad...

bors-servo · 2020-04-02T13:15:50Z

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing a651bad to master...

highfive assigned jdm Jan 6, 2020

highfive added the S-awaiting-review label Jan 6, 2020

highfive added the S-needs-rebase label Jan 6, 2020

teapotd mentioned this pull request Jan 8, 2020

Add bindings for RegExp flags servo/mozjs#223

Closed

servo-wpt-sync mentioned this pull request Jan 10, 2020

Form constraint validation web-platform-tests/wpt#21135

Merged

teapotd mentioned this pull request Jan 11, 2020

Simplify and fix iteration over radio button group #25490

Merged

3 of 4 tasks complete

teapotd marked this pull request as ready for review Jan 13, 2020

teapotd changed the title ~~[WIP] Form constraint validation~~ Form constraint validation Jan 13, 2020

jdm requested changes Jan 23, 2020

View changes

jdm added S-needs-code-changes S-needs-rebase and removed S-awaiting-review S-needs-rebase labels Jan 23, 2020

pshaughn mentioned this pull request Feb 7, 2020

form-associated custom elements and their ElementInternals #25705

Open

teapotd mentioned this pull request Mar 23, 2020

Add RegExp bindings servo/mozjs#237

Merged

highfive added S-awaiting-review and removed S-needs-code-changes labels Apr 1, 2020

highfive added S-tests-failed and removed S-awaiting-merge labels Apr 1, 2020

highfive added S-awaiting-merge and removed S-tests-failed labels Apr 2, 2020

highfive added S-tests-failed and removed S-awaiting-merge labels Apr 2, 2020

highfive added the S-needs-rebase label Apr 2, 2020

Form constraints validation

Loading status checks…

779552e

highfive added S-awaiting-review and removed S-tests-failed labels Apr 2, 2020

highfive added S-awaiting-merge and removed S-awaiting-review S-needs-rebase labels Apr 2, 2020

bors-servo merged commit a651bad into servo:master Apr 2, 2020
2 checks passed

2 checks passed

Community-TC (pull_request) TaskGroup: success
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Apr 2, 2020

teapotd deleted the teapotd:form-validation branch Apr 29, 2020

servo / servo

Form constraint validation #25447

Form constraint validation #25447

teapotd commented Jan 6, 2020 •

edited

highfive commented Jan 6, 2020

bors-servo commented Jan 6, 2020

servo-wpt-sync commented Jan 8, 2020

bors-servo commented Jan 9, 2020

servo-wpt-sync commented Jan 10, 2020

servo-wpt-sync commented Jan 11, 2020

servo-wpt-sync commented Jan 11, 2020

servo-wpt-sync commented Jan 12, 2020

servo-wpt-sync commented Jan 13, 2020

teapotd commented Jan 13, 2020 •

edited

servo-wpt-sync commented Jan 13, 2020

jdm left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors-servo commented Apr 1, 2020

jdm commented Apr 2, 2020

bors-servo commented Apr 2, 2020

bors-servo commented Apr 2, 2020

jdm commented Apr 2, 2020

bors-servo commented Apr 2, 2020

bors-servo commented Apr 2, 2020

servo-wpt-sync commented Apr 2, 2020

jdm commented Apr 2, 2020

bors-servo commented Apr 2, 2020

bors-servo commented Apr 2, 2020

bors-servo commented Apr 2, 2020

servo / servo

Join GitHub today

Form constraint validation #25447

Form constraint validation #25447

Conversation

teapotd commented Jan 6, 2020 • edited

highfive commented Jan 6, 2020

bors-servo commented Jan 6, 2020

servo-wpt-sync commented Jan 8, 2020

bors-servo commented Jan 9, 2020

servo-wpt-sync commented Jan 10, 2020

servo-wpt-sync commented Jan 11, 2020

servo-wpt-sync commented Jan 11, 2020

servo-wpt-sync commented Jan 12, 2020

servo-wpt-sync commented Jan 13, 2020

teapotd commented Jan 13, 2020 • edited

servo-wpt-sync commented Jan 13, 2020

jdm left a comment

This comment has been minimized.

jdm Jan 23, 2020

This comment has been minimized.

teapotd Jan 23, 2020

This comment has been minimized.

teapotd Apr 1, 2020

bors-servo commented Apr 1, 2020

jdm commented Apr 2, 2020

bors-servo commented Apr 2, 2020

bors-servo commented Apr 2, 2020

jdm commented Apr 2, 2020

bors-servo commented Apr 2, 2020

bors-servo commented Apr 2, 2020

servo-wpt-sync commented Apr 2, 2020

jdm commented Apr 2, 2020

bors-servo commented Apr 2, 2020

bors-servo commented Apr 2, 2020

bors-servo commented Apr 2, 2020

teapotd commented Jan 6, 2020 •

edited

teapotd commented Jan 13, 2020 •

edited