Submit inputs should be subject to constraint validation and match :valid/:invalid as needed

RESOLVED FIXED in mozilla32

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: bz, Assigned: agi)

Tracking

unspecified
mozilla32
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 6 obsolete attachments)

This is basically about backing out the changes from bug 606491, since the relevant spec bug got wontfixed.
(Assignee)

Updated

4 years ago
Assignee: nobody → agi.novanta
Status: NEW → ASSIGNED
(Assignee)

Comment 1

4 years ago
Created attachment 8405890 [details] [diff] [review]
Submit inputs are subject to constraint validation and match :valid/:invalid as needed
Attachment #8405890 - Flags: review?(bzbarsky)
(Assignee)

Updated

4 years ago
Attachment #8405890 - Attachment description: Test changes to allow submit imputs to be subject to constraint validation ; r=bz → Submit inputs are subject to constraint validation and match :valid/:invalid as needed
(Assignee)

Comment 2

4 years ago
Created attachment 8405891 [details] [diff] [review]
Test changes to allow submit imputs to be subject to constraint validation ; r=bz

OK. I should have addressed everything. I did basic testing on my machine and everything seems fine.

Try Push:
https://tbpl.mozilla.org/?tree=Try&rev=7d7a58215c18

Thank you!
Attachment #8405891 - Flags: review?(bzbarsky)
(Assignee)

Comment 3

4 years ago
Created attachment 8405970 [details] [diff] [review]
Submit inputs are subject to constraint validation and match :valid/:invalid as needed  ; r=bz

Added a call to UpdateState in AfterSetAttr for aName == nsGkAtoms::disabled. Found this bug adding old tests.
Attachment #8405890 - Attachment is obsolete: true
Attachment #8405890 - Flags: review?(bzbarsky)
Attachment #8405970 - Flags: review?(bzbarsky)
(Assignee)

Comment 4

4 years ago
Created attachment 8405971 [details] [diff] [review]
Test changes to allow submit imputs to be subject to constraint validation ; r=bz

Ok added old tests and fixed the broken ones. 

I'm sorry for all this emails!

Thank you!
Attachment #8405891 - Attachment is obsolete: true
Attachment #8405891 - Flags: review?(bzbarsky)
Attachment #8405971 - Flags: review?(bzbarsky)
Comment on attachment 8405970 [details] [diff] [review]
Submit inputs are subject to constraint validation and match :valid/:invalid as needed  ; r=bz

Thank you for working on this!

>+++ b/content/html/content/src/HTMLButtonElement.cpp
>@@ -392,27 +414,30 @@ HTMLButtonElement::BindToTree(nsIDocumen
>+  UpdateBarredFromConstraintValidation();

Why is this call needed?

> HTMLButtonElement::UnbindFromTree(bool aDeep, bool aNullParent)
>+  UpdateBarredFromConstraintValidation();

Likewise.

> HTMLButtonElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
>-      }
>+      } 

Please don't add that space.

>+++ b/content/html/content/src/HTMLButtonElement.h
>+  virtual void FieldSetDisabledChanged(bool aNotify); 

MOZ_OVERRIDE

r=me with the above dealt with
Attachment #8405970 - Flags: review?(bzbarsky) → review+
Comment on attachment 8405971 [details] [diff] [review]
Test changes to allow submit imputs to be subject to constraint validation ; r=bz

r=me assuming the tests pass.  But please fold the two patches together before pushing or asking for checkin-needed, ok?
Attachment #8405971 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 7

4 years ago
Created attachment 8409547 [details] [diff] [review]
Submit inputs are subject to constraint validation and match :valid/:invalid as needed ; r=bz

> Thank you for working on this!

My pleasure! I learnt a lot about testing thanks to this patch.

> Why is this call needed?

That call is not needed, thanks for catching this! I was fooled by the implementation of HTMLInputElement but apparently UpdateBarredFromConstraintValidation is already called from FieldSetDisabledChanged when the button is attached to a disabled fieldset.

I fixed some tests that were failing on Fennec (basically all the -disabled ones) because on mobile a disabled button has a slightly different style than a non-disabled one when styled with "background", apparently.

Try push: https://tbpl.mozilla.org/?tree=Try&rev=6e8efa3fcd75

Thanks!
Attachment #8405970 - Attachment is obsolete: true
Attachment #8405971 - Attachment is obsolete: true
Attachment #8409547 - Flags: review?(bzbarsky)
Comment on attachment 8409547 [details] [diff] [review]
Submit inputs are subject to constraint validation and match :valid/:invalid as needed ; r=bz

Sorry for the terrible lag; I was out most of last week.  :(

r=me, but please fix the commit message before pushing.
Attachment #8409547 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 9

4 years ago
Created attachment 8414945 [details] [diff] [review]
submit.patch

No problem at all! Thank you for the review Boris. 

Fixed the message and carried over the review+. Ready to land!

Thank you.
Attachment #8409547 - Attachment is obsolete: true
Attachment #8414945 - Flags: review+
(Assignee)

Comment 10

4 years ago
Created attachment 8414946 [details] [diff] [review]
Submit inputs are subject to constraint validation and match :valid/:invalid as needed

Mh. Something wrong happened. Now it should be fine.
Attachment #8414945 - Attachment is obsolete: true
Attachment #8414946 - Flags: review+
Looks lovely.  Thanks!
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7f1eda6d68cb
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32

Updated

3 years ago
Blocks: 1165264
You need to log in before you can comment on or make changes to this bug.