982039 - Radio input required not working when element disabled by js

Status: RESOLVED FIXED

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

Description

[Markus Doits]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release)
Build ID: 20140306171728

Steps to reproduce:

When a required radio input disabled by js after the page loaded, the required check is not working and submitting the form even nothing is checked.

See http://jsfiddle.net/3Xa4g/13/ - simply click on submit and the form is submitted even the radio input is not checked.



Actual results:

The form is submitted even no radio input is checked.


Expected results:

The form should be marked as invalid and not submitted until a radio button is checked.

Comment 1

[Liz Henry (:lizzard) (relman/hg->git project)]

Confirmed on Firefox 28 and on Nightly as well. 
Works as I'd expect it to in Chrome, but not in Safari. 

I'm not sure if this is an issue for HTML Form submission, or something else.

Comment 2

[Boris Zbarsky [:bzbarsky]]

This is a bit exciting.

HTMLInputElement::UpdateValueMissingValidityStateForRadio has this bit:

  valueMissing = IsMutable() && required && !selected;

Of course in this case, IsMutable() is false.  And then it does:

  container->SetValueMissingState(name, valueMissing);

What we should probably do instead is just treat disabled+required radios as not required in HTMLFormElement::RadioRequiredChanged, invoke that when @disabled changes, and then drop the IsMutable() bits in UpdateValueMissingValidityStateForRadio, since those will no longer be necessary.  Olli, would that make sense?

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Or this way.

https://tbpl.mozilla.org/?tree=Try&rev=5860a4db4c24

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8453361 [details] [diff] [review]
patch

Hmm.  I _think_ this works...

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Bah,  ###!!! ASSERTION: mRequiredRadioCount about to wrap below 0!: 'radioGroup->mRequiredRadioCount != 0'

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Oh, wait, that is an existing bug.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

That is explained in https://bugzilla.mozilla.org/show_bug.cgi?id=940203#c102

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Attachment: +small hack to make sure we shouldn't trigger that existing bug

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

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

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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