Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make stepUp, stepDown, valueAsNumber, valueAsDate, and list work in HTMLInputElement #25405

Merged
merged 1 commit into from Jan 11, 2020

Conversation

@pshaughn
Copy link
Member

pshaughn commented Dec 30, 2019

Steppy operation of input elements now functions. A few related tests still fail because of incorrect test expectations. A couple range reftests fail on what might be correct expectations; solving that involves layout, not DOM.
Main changes here:

  • str.rs now has its datetime format parsers public, with some changes to align their output with chrono::NaiveDateTime and fix a bug in milliseconds validation
  • HTMLInputElement has new attributes, and code behind them for relevant concepts
  • servoparser/mod.rs now has some extra code to ensure that input sanitization happens at the right moment during input element creation; the spec is non-specific about when this happens, but it has to happen somewhere and in #21952 @Eijebong had independently come to the same conclusion as me about the timing.
  • Some tests are added where WPT wasn't already testing functionality I was adding; Firefox and Chrome pass them, Safari fails some in a reasonably expected way
  • Lots of test metadata changed to passing

One test continues to fail because 0.1 * 6 != 0.6 and the spec is written in abstractions instead of actual floating-point numbers; other test failures are WPT bugs I've reported as such and will write fixes for soon if no one else does.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #25390 and fix #25388 and fix #19773 (except validation doesn't exist) and fix #25386
  • There are tests for these changes OR
@highfive
Copy link

highfive commented Dec 30, 2019

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/webidls/HTMLInputElement.webidl, components/script/dom/element.rs, components/script/dom/servoparser/mod.rs, components/script/dom/htmlinputelement.rs, components/script/dom/bindings/str.rs
  • @KiChjang: components/script/dom/webidls/HTMLInputElement.webidl, components/script/dom/element.rs, components/script/dom/servoparser/mod.rs, components/script/dom/htmlinputelement.rs, components/script/dom/bindings/str.rs
@pshaughn
Copy link
Member Author

pshaughn commented Dec 30, 2019

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Dec 30, 2019

Trying commit b0be07b with merge f657e0a...

bors-servo added a commit that referenced this pull request Dec 30, 2019
stepUp, stepDown, and valueAsNumber work [draft]

<!-- Please describe your changes on the following line: -->
Steppy operation of input elements now functions. A few related tests still fail because of incorrect test expectations. A couple range reftests fail on what might be correct expectations; solving that involves layout, not DOM.
#25386 still needs valueAsDate and list; I'll probably update that within this PR since it's going to have changes in common with this.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25390 and fix #25388 and fix #19773 (except validation doesn't exist) and progress #25386

<!-- Either: -->
- [X] There are tests for these changes OR

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Dec 30, 2019

💔 Test failed - status-taskcluster

@pshaughn
Copy link
Member Author

pshaughn commented Dec 31, 2019

@bors-servo retry=wpt

@pshaughn
Copy link
Member Author

pshaughn commented Dec 31, 2019

is that the wrong syntax?
@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Dec 31, 2019

Trying commit 6733699 with merge 7544c1b...

bors-servo added a commit that referenced this pull request Dec 31, 2019
stepUp, stepDown, and valueAsNumber work [draft]

<!-- Please describe your changes on the following line: -->
Steppy operation of input elements now functions. A few related tests still fail because of incorrect test expectations. A couple range reftests fail on what might be correct expectations; solving that involves layout, not DOM.
#25386 still needs valueAsDate and list; I'll probably update that within this PR since it's going to have changes in common with this.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25390 and fix #25388 and fix #19773 (except validation doesn't exist) and progress #25386

<!-- Either: -->
- [X] There are tests for these changes OR

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Dec 31, 2019

💔 Test failed - status-taskcluster

@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Dec 31, 2019

Opened new PR for upstreamable changes.

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

@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Dec 31, 2019

Transplanted upstreamable changes to existing PR.

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

@pshaughn
Copy link
Member Author

pshaughn commented Dec 31, 2019

List is working now, and I added a test for it... existing valueAsNumber tests aren't covering the chrono types, I'm gonna go in and add those too, and this is still a draft
@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Dec 31, 2019

Trying commit bb4c34a with merge f138fc8...

bors-servo added a commit that referenced this pull request Dec 31, 2019
stepUp, stepDown, and valueAsNumber work [draft]

<!-- Please describe your changes on the following line: -->
Steppy operation of input elements now functions. A few related tests still fail because of incorrect test expectations. A couple range reftests fail on what might be correct expectations; solving that involves layout, not DOM.
#25386 still needs valueAsDate and list; I'll probably update that within this PR since it's going to have changes in common with this.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25390 and fix #25388 and fix #19773 (except validation doesn't exist) and progress #25386

<!-- Either: -->
- [X] There are tests for these changes OR

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Dec 31, 2019

💔 Test failed - status-taskcluster

@pshaughn pshaughn force-pushed the pshaughn:minmaxstep branch from bb4c34a to 1ebf15e Dec 31, 2019
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Dec 31, 2019

Transplanted upstreamable changes to existing PR.

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

@pshaughn
Copy link
Member Author

pshaughn commented Dec 31, 2019

Don't bother trying that push, it needs a rebase (because a WPT test was fixed upstream) and a bunch of idlharness test metadata.

@pshaughn pshaughn force-pushed the pshaughn:minmaxstep branch from 1ebf15e to 2998ac6 Dec 31, 2019
@pshaughn
Copy link
Member Author

pshaughn commented Dec 31, 2019

I think this might be all synced up now.
@bors-servo try=wpt

@pshaughn pshaughn force-pushed the pshaughn:minmaxstep branch from cc33044 to 87e86c8 Jan 11, 2020
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Jan 11, 2020

Transplanted upstreamable changes to existing PR.

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

@pshaughn
Copy link
Member Author

pshaughn commented Jan 11, 2020

Sorry about that; the new tests from upstream led me to fix an actual bug despite the time-zone and optional-seconds test inconsistencies (one of my codepaths was multiplying seconds by 3600 instead of hours)
@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Jan 11, 2020

Trying commit 87e86c8 with merge 94d7c9e...

bors-servo added a commit that referenced this pull request Jan 11, 2020
make stepUp, stepDown, valueAsNumber, valueAsDate, and list work in HTMLInputElement

<!-- Please describe your changes on the following line: -->
Steppy operation of input elements now functions. A few related tests still fail because of incorrect test expectations. A couple range reftests fail on what might be correct expectations; solving that involves layout, not DOM.
Main changes here:
- str.rs now has its datetime format parsers public, with some changes to align their output with chrono::NaiveDateTime and fix a bug in milliseconds validation
- HTMLInputElement has new attributes, and code behind them for relevant concepts
- servoparser/mod.rs now has some extra code to ensure that input sanitization happens at the right moment during input element creation; the spec is non-specific about when this happens, but it has to happen somewhere and in #21952 @Eijebong had independently come to the same conclusion as me about the timing.
- Some tests are added where WPT wasn't already testing functionality I was adding; Firefox and Chrome pass them, Safari fails some in a reasonably expected way
- Lots of test metadata changed to passing

One test continues to fail because 0.1 * 6 != 0.6 and the spec is written in abstractions instead of actual floating-point numbers; other test failures are WPT bugs I've reported as such and will write fixes for soon if no one else does.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25390 and fix #25388 and fix #19773 (except validation doesn't exist) and fix #25386

<!-- Either: -->
- [X] There are tests for these changes OR

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 11, 2020

☀️ Test successful - status-taskcluster
State: approved= try=True

@jdm
Copy link
Member

jdm commented Jan 11, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Jan 11, 2020

📌 Commit 87e86c8 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Jan 11, 2020

Testing commit 87e86c8 with merge efc38de...

bors-servo added a commit that referenced this pull request Jan 11, 2020
make stepUp, stepDown, valueAsNumber, valueAsDate, and list work in HTMLInputElement

<!-- Please describe your changes on the following line: -->
Steppy operation of input elements now functions. A few related tests still fail because of incorrect test expectations. A couple range reftests fail on what might be correct expectations; solving that involves layout, not DOM.
Main changes here:
- str.rs now has its datetime format parsers public, with some changes to align their output with chrono::NaiveDateTime and fix a bug in milliseconds validation
- HTMLInputElement has new attributes, and code behind them for relevant concepts
- servoparser/mod.rs now has some extra code to ensure that input sanitization happens at the right moment during input element creation; the spec is non-specific about when this happens, but it has to happen somewhere and in #21952 @Eijebong had independently come to the same conclusion as me about the timing.
- Some tests are added where WPT wasn't already testing functionality I was adding; Firefox and Chrome pass them, Safari fails some in a reasonably expected way
- Lots of test metadata changed to passing

One test continues to fail because 0.1 * 6 != 0.6 and the spec is written in abstractions instead of actual floating-point numbers; other test failures are WPT bugs I've reported as such and will write fixes for soon if no one else does.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25390 and fix #25388 and fix #19773 (except validation doesn't exist) and fix #25386

<!-- Either: -->
- [X] There are tests for these changes OR

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 11, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Jan 11, 2020

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Jan 11, 2020

Testing commit 87e86c8 with merge 63cb13f...

bors-servo added a commit that referenced this pull request Jan 11, 2020
make stepUp, stepDown, valueAsNumber, valueAsDate, and list work in HTMLInputElement

<!-- Please describe your changes on the following line: -->
Steppy operation of input elements now functions. A few related tests still fail because of incorrect test expectations. A couple range reftests fail on what might be correct expectations; solving that involves layout, not DOM.
Main changes here:
- str.rs now has its datetime format parsers public, with some changes to align their output with chrono::NaiveDateTime and fix a bug in milliseconds validation
- HTMLInputElement has new attributes, and code behind them for relevant concepts
- servoparser/mod.rs now has some extra code to ensure that input sanitization happens at the right moment during input element creation; the spec is non-specific about when this happens, but it has to happen somewhere and in #21952 @Eijebong had independently come to the same conclusion as me about the timing.
- Some tests are added where WPT wasn't already testing functionality I was adding; Firefox and Chrome pass them, Safari fails some in a reasonably expected way
- Lots of test metadata changed to passing

One test continues to fail because 0.1 * 6 != 0.6 and the spec is written in abstractions instead of actual floating-point numbers; other test failures are WPT bugs I've reported as such and will write fixes for soon if no one else does.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25390 and fix #25388 and fix #19773 (except validation doesn't exist) and fix #25386

<!-- Either: -->
- [X] There are tests for these changes OR

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 11, 2020

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

@bors-servo bors-servo merged commit 87e86c8 into servo:master Jan 11, 2020
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
bors-servo added a commit that referenced this pull request Apr 1, 2020
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 added a commit that referenced this pull request Apr 1, 2020
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 added a commit that referenced this pull request Apr 1, 2020
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 added a commit that referenced this pull request Apr 2, 2020
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 added a commit that referenced this pull request Apr 2, 2020
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.