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

Issue 2355743005: Queue a close event for dialog.close (Closed)

Created:
4 years, 3 months ago by xing.xu
Modified:
4 years, 2 months ago
Reviewers:
skobes
CC:
chromium-reviews, blink-reviews, dglazkov+blink, blink-reviews-html_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Queue a close event for dialog.close Currently dialog synchronously dispatches the close event. While according to https://www.w3.org/TR/2013/CR-html5-20130806/interactive-elements.html#close-the-dialog, dialog should "Queue a task to fire a simple event named close at subject". So this fix queues the close event instead of dispatching it immediately. Layout test dialog-close-bug380087.html is from http://w3c-test.org/html/semantics/interactive-elements/the-dialog-element/dialog-close.html. BUG=380087 TEST=Load http://w3c-test.org/html/semantics/interactive-elements/the-dialog-element/dialog-close.html R=skobes@chromium.org Committed: https://crrev.com/9027b8baa9ca3ada4de5e8a9efd28ee7ef476c0f Cr-Commit-Position: refs/heads/master@{#420839}

Patch Set 1 #

Patch Set 2 : Queue a close event for dialog.close #

Patch Set 3 : Queue a close event for dialog.close #

Patch Set 4 : Queue a close event for dialog.close #

Patch Set 5 : Queue a close event for dialog.close #

Messages

Total messages: 31 (18 generated)
xing.xu
Hi, please help to review
4 years, 3 months ago (2016-09-21 03:23:16 UTC) #3
skobes
Can you add a layout test? Also the spec link in the bug is broken, ...
4 years, 3 months ago (2016-09-21 17:41:00 UTC) #4
xing.xu
@skobes, Layout test added and description changed.
4 years, 2 months ago (2016-09-23 02:23:09 UTC) #5
skobes
Thanks. We don't usually include the bug # in the test filename. I'd call it ...
4 years, 2 months ago (2016-09-23 15:36:00 UTC) #14
xing.xu
@skobes, rename is done.
4 years, 2 months ago (2016-09-23 23:38:10 UTC) #15
skobes
lgtm
4 years, 2 months ago (2016-09-23 23:45:27 UTC) #16
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/2355743005/60001
4 years, 2 months ago (2016-09-23 23:55:49 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/303534)
4 years, 2 months ago (2016-09-24 00:57:56 UTC) #20
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/2355743005/80001
4 years, 2 months ago (2016-09-24 07:31:46 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_chromium_compile_only_ng/builds/205417)
4 years, 2 months ago (2016-09-24 08:17:10 UTC) #25
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/2355743005/80001
4 years, 2 months ago (2016-09-24 09:06:59 UTC) #27
commit-bot: I haz the power
Committed patchset #5 (id:80001)
4 years, 2 months ago (2016-09-24 09:38:58 UTC) #29
commit-bot: I haz the power
4 years, 2 months ago (2016-09-24 09:41:11 UTC) #31
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/9027b8baa9ca3ada4de5e8a9efd28ee7ef476c0f
Cr-Commit-Position: refs/heads/master@{#420839}

Powered by Google App Engine
This is Rietveld 408576698