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

Implements basic form resetting #4133

Merged
merged 12 commits into from Dec 16, 2014
Merged

Implements basic form resetting #4133

merged 12 commits into from Dec 16, 2014

Conversation

@mttr
Copy link
Contributor

mttr commented Nov 28, 2014

We can reset <input type=text> fields! I wish I could've done something with checkboxes, but unfortunately, that's it for now.

In addition to that, this PR implements HTMLInputAttribute.defaultValue, updates wpt-test to expect passing tests as a result of that implementation, and fixes an index error crash with text inputs.

edit: also includes an html example where one may lazily watch form resets in action: tests/html/form_reset_handsfree.html

@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Nov 28, 2014

Critic review: https://critic.hoppipolla.co.uk/r/3309

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@Manishearth
Copy link
Member

Manishearth commented Dec 8, 2014

This looks good from my phone, I'll review this in detail soon.

@Manishearth Manishearth self-assigned this Dec 8, 2014
@mttr mttr force-pushed the mttr:form_resetting branch from 6e32102 to 3871225 Dec 8, 2014
@mttr
Copy link
Contributor Author

mttr commented Dec 8, 2014

Also implemented the :indeterminate pseudo-class while I was in there, which fixes #4079.

I may have broken critic, though..

mttr added 2 commits Nov 27, 2014
...and changes SetValue to update the input text instead of the content
attr.

Also includes a comment summarizing everything I currently know with
respect to an input elements checkedness vs its IDL attributes vs its
content attributes.
@mttr mttr force-pushed the mttr:form_resetting branch 2 times, most recently from 6379b8f to a425a39 Dec 16, 2014
mttr added 9 commits Nov 27, 2014
What can this do? Reset `<input type=text>` fields back to their default
value through a call to a form's reset method. That's all for now!

Fixes compile error after rebase
Also modified tests/html/textarea.html to allow for the testing of the
textarea's dirty value flag.
And modifies test-inputs.html to test.

Fixes wpt breaking mistake
Addresses reviews
@mttr mttr force-pushed the mttr:form_resetting branch from a425a39 to 504f968 Dec 16, 2014
@mttr
Copy link
Contributor Author

mttr commented Dec 16, 2014

Rebased and squashed!

@Manishearth

This comment has been minimized.

Copy link

Manishearth commented on 504f968 Dec 16, 2014

r+

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented on 504f968 Dec 16, 2014

saw approval from Manishearth
at mttr@504f968

This comment has been minimized.

Copy link
Contributor

bors-servo replied Dec 16, 2014

merging mttr/servo/form_resetting = 504f968 into auto

This comment has been minimized.

Copy link
Contributor

bors-servo replied Dec 16, 2014

mttr/servo/form_resetting = 504f968 merged ok, testing candidate = ae5699a

This comment has been minimized.

Copy link
Contributor

bors-servo replied Dec 16, 2014

bors-servo pushed a commit that referenced this pull request Dec 16, 2014
We can reset `<input type=text>` fields! I wish I could've done something with checkboxes, but unfortunately, that's it for now.

In addition to that, this PR implements `HTMLInputAttribute.defaultValue`, updates wpt-test to expect passing tests as a result of that implementation, and fixes an index error crash with text inputs.

edit: also includes an html example where one may lazily watch form resets in action: ` tests/html/form_reset_handsfree.html`
@jdm
Copy link
Member

jdm commented Dec 16, 2014

/html/semantics/forms/the-input-element/password.html
-----------------------------------------------------
PASS expected FAIL Setting value changes the current value for password, but not the value content attribute
/html/semantics/forms/the-input-element/reset.html
--------------------------------------------------
PASS expected FAIL reset button only resets the form owner
PASS expected FAIL clicking on a disabled reset does nothing
PASS expected FAIL reset button associated with a form using the form attribute resets all the form's controls
/html/semantics/selectors/pseudo-classes/indeterminate.html
-----------------------------------------------------------
PASS expected FAIL adding a value to progress1 should put it in a determinate state
@mttr
Copy link
Contributor Author

mttr commented Dec 16, 2014

Got it.

@jdm

This comment has been minimized.

Copy link

jdm commented on e496386 Dec 16, 2014

r+

@jdm jdm added S-awaiting-merge and removed S-tests-failed labels Dec 16, 2014
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented on e496386 Dec 16, 2014

saw approval from jdm
at mttr@e496386

This comment has been minimized.

Copy link
Contributor

bors-servo replied Dec 16, 2014

merging mttr/servo/form_resetting = e496386 into auto

This comment has been minimized.

Copy link
Contributor

bors-servo replied Dec 16, 2014

mttr/servo/form_resetting = e496386 merged ok, testing candidate = 5951056

This comment has been minimized.

Copy link
Contributor

bors-servo replied Dec 16, 2014

fast-forwarding master to auto = 5951056

bors-servo pushed a commit that referenced this pull request Dec 16, 2014
We can reset `<input type=text>` fields! I wish I could've done something with checkboxes, but unfortunately, that's it for now.

In addition to that, this PR implements `HTMLInputAttribute.defaultValue`, updates wpt-test to expect passing tests as a result of that implementation, and fixes an index error crash with text inputs.

edit: also includes an html example where one may lazily watch form resets in action: ` tests/html/form_reset_handsfree.html`
@bors-servo bors-servo closed this Dec 16, 2014
@bors-servo bors-servo merged commit e496386 into servo:master Dec 16, 2014
1 check passed
1 check passed
default all tests passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.