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

Issue 2771193002: Form validation: Validation bubble should be closed on document unload process. (Closed)

Created:
3 years, 9 months ago by tkent
Modified:
3 years, 9 months ago
Reviewers:
keishi
CC:
blink-reviews, blink-reviews-dom_chromium.org, blink-reviews-html_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, kinuko+watch, rwlbuis, sof
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Form validation: Validation bubble should be closed on document unload process. This CL fixes a bug that a validation bubble is not closed by page navigation in some cases. We close a validation message on Page::documentDetached(). However it seems it was too late to communicate with the browser process in some cases. So, this CL moves it to Document unload timing. * Add ValidationMessage::willUnloadDocument(), which closes a validation bubble, and Document::dispatchUnloadEvents() calls it indirectly through Page. * HTMLFormControlElement prevents from showing a validation message after the unload processing. BUG=704560 Review-Url: https://codereview.chromium.org/2771193002 Cr-Commit-Position: refs/heads/master@{#459701} Committed: https://chromium.googlesource.com/chromium/src/+/a896ff44a395a50ab18f5120f20b7eb5a9550247

Patch Set 1 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+94 lines, -2 lines) Patch
M third_party/WebKit/Source/core/dom/Document.cpp View 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/DocumentTest.cpp View 3 chunks +77 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLFormControlElement.cpp View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/page/Page.h View 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/page/Page.cpp View 1 chunk +5 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/page/ValidationMessageClient.h View 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/web/ValidationMessageClientImpl.h View 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/web/ValidationMessageClientImpl.cpp View 1 chunk +6 lines, -1 line 0 comments Download

Messages

Total messages: 23 (17 generated)
tkent
keishi@, would you review this please?
3 years, 9 months ago (2017-03-24 08:07:39 UTC) #11
keishi
LGTM
3 years, 9 months ago (2017-03-27 01:21:24 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2771193002/20001
3 years, 9 months ago (2017-03-27 01:22:26 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/391565)
3 years, 9 months ago (2017-03-27 02:32:30 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2771193002/20001
3 years, 9 months ago (2017-03-27 03:00:44 UTC) #20
commit-bot: I haz the power
3 years, 9 months ago (2017-03-27 03:48:12 UTC) #23
Message was sent while issue was closed.
Committed patchset #1 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/a896ff44a395a50ab18f5120f20b...

Powered by Google App Engine
This is Rietveld 408576698