Clicking a button on disconnected form should not fire submit event.

Clicking a button on disconnected form should not fire submit event.

Add a isConnected() check in HTMLFormElement::PrepareForSubmission
to avoid firing the submit events.

BUG=723004
Review-Url: https://codereview.chromium.org/2900943002
Cr-Commit-Position: refs/heads/master@{#474476}
Committed: https://chromium.googlesource.com/chromium/src/+/cad664141c2aed77dd0049928c1c9d004cce72a0

[Shanmuga Pandi, 2017-05-23 06:41:34]

shanmuga.m@samsung.com changed reviewers:
+ foolip@chromium.org, fs@opera.com, tkent@chromium.org

[Shanmuga Pandi, 2017-05-23 06:41:34]

PTAL!!

Thanks

[tkent, 2017-05-23 06:55:26]

https://codereview.chromium.org/2900943002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLButtonElement.cpp (right):

https://codereview.chromium.org/2900943002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLButtonElement.cpp:108: if
(!isConnected())
Here is not a right place to check this.
We have a similar issue with <input type=submit>.

HTMLFormElement::PrepareForSubmission() is the right place, I think.

[Shanmuga Pandi, 2017-05-23 10:33:02]

Description was changed from

==========
Clicking a button on disconnected form should not fire submit event.

Add a isConnected() check in HTMLButtonElement::DefaultEventHandler
to avoid firing the submit events.

BUG=723004
==========

to

==========
Clicking a button on disconnected form should not fire submit event.

Add a isConnected() check in HTMLFormElement::PrepareForSubmission
to avoid firing the submit events.

BUG=723004
==========

[Shanmuga Pandi, 2017-05-23 10:34:05]

https://codereview.chromium.org/2900943002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLButtonElement.cpp (right):

https://codereview.chromium.org/2900943002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLButtonElement.cpp:108: if
(!isConnected())
On 2017/05/23 06:55:26, tkent wrote:
> Here is not a right place to check this.
> We have a similar issue with <input type=submit>.
> 
> HTMLFormElement::PrepareForSubmission() is the right place, I think.

Done.

[tkent, 2017-05-23 23:32:52]

https://codereview.chromium.org/2900943002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLFormElement.cpp (right):

https://codereview.chromium.org/2900943002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLFormElement.cpp:287: (submit_button &&
!submit_button->isConnected()))
You don't need to check isConnected() of submit_button. isConnected() for |this|
is enough because their connectedness is always same.
Please show a warning message same as HTMLFormElement::Submit().

Note that the isConnected() check in Submit() should be moved to here, but we
can't do it now because PrepareForSubmission() is not always called.

[Shanmuga Pandi, 2017-05-24 07:09:54]

https://codereview.chromium.org/2900943002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLFormElement.cpp (right):

https://codereview.chromium.org/2900943002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLFormElement.cpp:287: (submit_button &&
!submit_button->isConnected()))
On 2017/05/23 23:32:52, tkent wrote:
> You don't need to check isConnected() of submit_button. isConnected() for
|this|
> is enough because their connectedness is always same.
> Please show a warning message same as HTMLFormElement::Submit().
> 
> Note that the isConnected() check in Submit() should be moved to here, but we
> can't do it now because PrepareForSubmission() is not always called.

Done.

[Shanmuga Pandi, 2017-05-24 07:10:45]

The CQ bit was checked by shanmuga.m@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-24 07:11:11]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-24 09:24:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-24 09:24:48]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[foolip, 2017-05-24 11:58:28]

foolip@chromium.org changed reviewers:
- foolip@chromium.org

[foolip, 2017-05-24 11:58:29]

I'll be OOO for a while, leaving this to the other reviewers.

[tkent, 2017-05-24 22:35:09]

The CQ bit was checked by tkent@chromium.org

[tkent, 2017-05-24 22:35:10]

lgtm

[commit-bot: I haz the power, 2017-05-24 22:35:53]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-24 23:40:32]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495665309605920,
"parent_rev": "a0646d775e805babb8d6949f955071011df709f2", "commit_rev":
"cad664141c2aed77dd0049928c1c9d004cce72a0"}

[commit-bot: I haz the power, 2017-05-24 23:40:50]

Description was changed from

==========
Clicking a button on disconnected form should not fire submit event.

Add a isConnected() check in HTMLFormElement::PrepareForSubmission
to avoid firing the submit events.

BUG=723004
==========

to

==========
Clicking a button on disconnected form should not fire submit event.

Add a isConnected() check in HTMLFormElement::PrepareForSubmission
to avoid firing the submit events.

BUG=723004

Review-Url: https://codereview.chromium.org/2900943002
Cr-Commit-Position: refs/heads/master@{#474476}
Committed:
https://chromium.googlesource.com/chromium/src/+/cad664141c2aed77dd0049928c1c...
==========

[commit-bot: I haz the power, 2017-05-24 23:40:51]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/cad664141c2aed77dd0049928c1c...