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

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

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

Comment 1

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Submit inputs are subject to constraint validation and match :valid/:invalid as needed

Comment 2

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: 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!

Comment 3

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: 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.

Comment 4

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: 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!

Comment 5

[Boris Zbarsky [: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

Comment 6

[Boris Zbarsky [:bzbarsky]]

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?

Comment 7

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: 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!

Comment 8

[Boris Zbarsky [: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.

Comment 9

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: 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.

Comment 10

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Submit inputs are subject to constraint validation and match :valid/:invalid as needed

Mh. Something wrong happened. Now it should be fine.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Looks lovely.  Thanks!

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7f1eda6d68cb

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7f1eda6d68cb