Fix the issue where checking a form for :valid/:invalid would trigger invalid events for its elemen…

Fix the issue where checking a form for :valid/:invalid would trigger invalid events for its elements (if set).

BUG=360466
TEST=added a case to the layout test, to test for the issue

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183862

[Bartek Nowierski, 2014-10-16 09:03:30]

bartekn@chromium.org changed reviewers:
+ keishi@chromium.org, tkent@chromium.org

[keishi, 2014-10-17 04:52:37]

https://codereview.chromium.org/658063002/diff/1/LayoutTests/fast/forms/form-...
File LayoutTests/fast/forms/form-pseudo-valid-style.html (right):

https://codereview.chromium.org/658063002/diff/1/LayoutTests/fast/forms/form-...
LayoutTests/fast/forms/form-pseudo-valid-style.html:33: parent.innerHTML =
'<form id=form1><input type=text id=input1 required><input type=text id=input2
required value=a><input type=submit id=sub1></form>';
nit: unintentional change?

https://codereview.chromium.org/658063002/diff/1/Source/core/html/HTMLFormEle...
File Source/core/html/HTMLFormElement.h (right):

https://codereview.chromium.org/658063002/diff/1/Source/core/html/HTMLFormEle...
Source/core/html/HTMLFormElement.h:154: bool
checkInvalidControlsAndCollectUnhandled(WillBeHeapVector<RefPtrWillBeMember<FormAssociatedElement>
>*, bool);
http://www.chromium.org/blink/coding-style says "Prefer enums to bools on
function parameters if callers are likely to be passing constants". I think this
should be an enum like DispatchInvalidEvent.

[Bartek Nowierski, 2014-10-17 05:34:57]

https://codereview.chromium.org/658063002/diff/1/LayoutTests/fast/forms/form-...
File LayoutTests/fast/forms/form-pseudo-valid-style.html (right):

https://codereview.chromium.org/658063002/diff/1/LayoutTests/fast/forms/form-...
LayoutTests/fast/forms/form-pseudo-valid-style.html:33: parent.innerHTML =
'<form id=form1><input type=text id=input1 required><input type=text id=input2
required value=a><input type=submit id=sub1></form>';
On 2014/10/17 04:52:37, keishi wrote:
> nit: unintentional change?

Yes. I wanted to make it consistent, 'coz the rest of the file doesn't use ""
for attributes, and obviously it didn't make sense to make a separate change
just for that

https://codereview.chromium.org/658063002/diff/1/Source/core/html/HTMLFormEle...
File Source/core/html/HTMLFormElement.h (right):

https://codereview.chromium.org/658063002/diff/1/Source/core/html/HTMLFormEle...
Source/core/html/HTMLFormElement.h:154: bool
checkInvalidControlsAndCollectUnhandled(WillBeHeapVector<RefPtrWillBeMember<FormAssociatedElement>
>*, bool);
On 2014/10/17 04:52:37, keishi wrote:
> http://www.chromium.org/blink/coding-style says "Prefer enums to bools on
> function parameters if callers are likely to be passing constants". I think
this
> should be an enum like DispatchInvalidEvent.

Done.

[keishi, 2014-10-17 05:46:12]

https://codereview.chromium.org/658063002/diff/20001/Source/core/html/HTMLFor...
File Source/core/html/HTMLFormControlElement.h (right):

https://codereview.chromium.org/658063002/diff/20001/Source/core/html/HTMLFor...
Source/core/html/HTMLFormControlElement.h:91: enum DispatchInvalidEvent {
DispatchInvalidEventUnknown, DispatchInvalidEventEnabled,
DispatchInvalidEventDisabled };
We're not switching a enabled/disabled state so I think the names would be
better if we follow TextFieldEventBehavior's and name it something like
DispatchInvalidEvent and DispatchNoEvent. I don't think we need the Unknown
entry.

[Bartek Nowierski, 2014-10-17 06:17:59]

https://codereview.chromium.org/658063002/diff/20001/Source/core/html/HTMLFor...
File Source/core/html/HTMLFormControlElement.h (right):

https://codereview.chromium.org/658063002/diff/20001/Source/core/html/HTMLFor...
Source/core/html/HTMLFormControlElement.h:91: enum DispatchInvalidEvent {
DispatchInvalidEventUnknown, DispatchInvalidEventEnabled,
DispatchInvalidEventDisabled };
On 2014/10/17 05:46:12, keishi wrote:
> We're not switching a enabled/disabled state so I think the names would be
> better if we follow TextFieldEventBehavior's and name it something like
> DispatchInvalidEvent and DispatchNoEvent. I don't think we need the Unknown
> entry.

Your suggestion created a clash with DispatcNoEvent from TextFieldEventBehavior.
I resolved it by making longer names. Let me know if you like it.

[keishi, 2014-10-17 06:24:53]

LGTM

https://codereview.chromium.org/658063002/diff/40001/LayoutTests/fast/forms/f...
File LayoutTests/fast/forms/form-pseudo-valid-style.html (right):

https://codereview.chromium.org/658063002/diff/40001/LayoutTests/fast/forms/f...
LayoutTests/fast/forms/form-pseudo-valid-style.html:80: shouldBe('val', '"0"');
nit: For strings we try to use shouldBeEqualToString.

[Bartek Nowierski, 2014-10-17 06:52:24]

https://codereview.chromium.org/658063002/diff/40001/LayoutTests/fast/forms/f...
File LayoutTests/fast/forms/form-pseudo-valid-style.html (right):

https://codereview.chromium.org/658063002/diff/40001/LayoutTests/fast/forms/f...
LayoutTests/fast/forms/form-pseudo-valid-style.html:80: shouldBe('val', '"0"');
On 2014/10/17 06:24:53, keishi wrote:
> nit: For strings we try to use shouldBeEqualToString.

Done.

[tkent, 2014-10-17 06:55:21]

lgtm

[Bartek Nowierski, 2014-10-17 06:58:07]

The CQ bit was checked by bartekn@chromium.org

[commit-bot: I haz the power, 2014-10-17 06:58:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/658063002/60001

[commit-bot: I haz the power, 2014-10-17 08:16:29]

Committed patchset #4 (id:60001) as 183862