Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(40)

Issue 449823003: Form submitted from JS should not submit double values (Closed)

Created:
6 years, 4 months ago by Habib Virji
Modified:
6 years, 4 months ago
Reviewers:
keishi, tkent
CC:
blink-reviews, dglazkov+blink, blink-reviews-html_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Form submitted from JS should not submit double values Jquery validation adds hidden element and removes it after JS submit handler. In blink, it was resulting in extra value being submitted. This patch reverts changes of r170520, r171008 and r169158. r169158, changes are not needed anymore due to formcreate called from scheduleformsubmission. r170520 and r171008 removed variable which control double form submission when from JS and normal submission. Also fixes the issue with bug357101, which CL170520 fixed, by keeping validateInteractive independent of m_isSubmittingOrInUserJSSubmitEvent variable. R=tkent, keishi BUG=378949 TEST=Check add remove element from inside user JS event handler, does not lead to double value submission. Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=179776

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+81 lines, -1 line) Patch
A LayoutTests/fast/forms/submit-add-remove-element.html View 1 chunk +48 lines, -0 lines 2 comments Download
A LayoutTests/fast/forms/submit-add-remove-element-expected.txt View 1 chunk +11 lines, -0 lines 0 comments Download
M Source/core/html/HTMLFormElement.h View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/html/HTMLFormElement.cpp View 4 chunks +20 lines, -1 line 0 comments Download

Messages

Total messages: 7 (0 generated)
Habib Virji
Fixes the issue with form submission from User's JS onsubmit and normal submit handler.
6 years, 4 months ago (2014-08-07 12:58:28 UTC) #1
tkent
lgtm. Thank you for fixing this. https://codereview.chromium.org/449823003/diff/1/LayoutTests/fast/forms/submit-add-remove-element.html File LayoutTests/fast/forms/submit-add-remove-element.html (right): https://codereview.chromium.org/449823003/diff/1/LayoutTests/fast/forms/submit-add-remove-element.html#newcode38 LayoutTests/fast/forms/submit-add-remove-element.html:38: } nit: Need ...
6 years, 4 months ago (2014-08-08 00:49:33 UTC) #2
tkent
The CQ bit was checked by tkent@chromium.org
6 years, 4 months ago (2014-08-08 00:49:37 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/habib.virji@samsung.com/449823003/1
6 years, 4 months ago (2014-08-08 00:51:48 UTC) #4
commit-bot: I haz the power
Change committed as 179776
6 years, 4 months ago (2014-08-08 02:14:36 UTC) #5
Habib Virji
On 2014/08/08 00:49:33, tkent wrote: > lgtm. > > Thank you for fixing this. > ...
6 years, 4 months ago (2014-08-08 08:18:14 UTC) #6
tkent
6 years, 4 months ago (2014-08-08 08:21:18 UTC) #7
Message was sent while issue was closed.
On 2014/08/08 08:18:14, Habib Virji wrote:
> 
> Since this is already committed, is it okay to submit it as part of other CL?

No.

Powered by Google App Engine
This is Rietveld 408576698