Closed Bug 952982 Opened 6 years ago Closed 5 years ago

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

Categories

(Core :: DOM: Core & HTML, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: bzbarsky, Assigned: Agi)

References

Details

Attachments

(1 file, 6 obsolete files)

This is basically about backing out the changes from bug 606491, since the relevant spec bug got wontfixed.
Assignee: nobody → agi.novanta
Status: NEW → ASSIGNED
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
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)
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)
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+
> 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+
Attached patch submit.patch (obsolete) — Splinter Review
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+
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
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Blocks: 1165264
You need to log in before you can comment on or make changes to this bug.