New issue
Advanced search Search tips

Issue 883221 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression : Able to Check/Uncheck history items on background page even when 'Remove' confirmation dialog is active.

Reported by avsha...@etouch.net, Sep 12

Issue description

Chrome Version : 70.0.3538.16 (Official Build) 16ed95b41bb05e565b11fb66ac33c660b721f778-refs/branch-heads/3538@{#306} 32/64 bit
OS : Windows(7, 8, 8.1, 10), Linux(14.04 LTS), Mac(10.12.6, 10.13.1, 10.14, 10.13.6)

What steps will reproduce the problem?
1. Launch chrome and navigate to few web pages to generate some history records.
2. Navigate to chrome://history page, select few history entries and click on 'Delete' button present on toolbar. ('Remove' confirmation dialog appears)
3. Hit 'Ctrl + A' keys from keyboard while 'Delete' confirmation dialog is open.
4. Observe the background page.

Actual Result : Able to Check/Uncheck history items on background page even when 'Remove' confirmation dialog is active.

Expected Result : 'Ctrl + A' shortcut should not work on background page when 'Remove' confirmation dialog is currently active.

This is a regression issue, broken in M-66 and providing the 'per-revision' bisect info:
Good Build : 66.0.3334.0 (Revision : 532208)
Bad Build : 66.0.3335.0 (Revision : 533164)

Change Log URL :
https://chromium.googlesource.com/chromium/src/+log/18ac0a07d07f8687258809c8b98c678167e6ae7c..67c8f89613bfaf24bf52f303dbec8267b52c7831

Suspecting : https://chromium.googlesource.com/chromium/src/+/67c8f89613bfaf24bf52f303dbec8267b52c7831

@sangwoo : Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Note : Able to reproduce this issue in Stable #69.0.3497.92 and Canary build #71.0.3550.0
 
Actual_history.mp4
859 KB View Download
Expected_history.mp4
524 KB View Download
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab21d472ec0c8e9fcdc6933e485b828455011c4c

commit ab21d472ec0c8e9fcdc6933e485b828455011c4c
Author: sangwoo.ko <sangwoo.ko@navercorp.com>
Date: Mon Sep 24 19:01:36 2018

Don't propagate keydown events when dialog is open

We shouldn't propagate keydown events so that it can
behave modally.

Bug:  883221 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I4aa5257b4bf6c417123d659ba911cb01641a4cc7
Reviewed-on: https://chromium-review.googlesource.com/1230493
Reviewed-by: Scott Chen <scottchen@chromium.org>
Commit-Queue: Sang Woo Ko <sangwoo108@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593619}
[modify] https://crrev.com/ab21d472ec0c8e9fcdc6933e485b828455011c4c/AUTHORS
[modify] https://crrev.com/ab21d472ec0c8e9fcdc6933e485b828455011c4c/chrome/test/data/webui/md_bookmarks/command_manager_test.js
[modify] https://crrev.com/ab21d472ec0c8e9fcdc6933e485b828455011c4c/ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js

Status: Fixed (was: Assigned)
Hello, avshaikh@. A CL to fix this was landed. Could you verify this, please?
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5865a897f8bd64814275c2a7e90a9ad44b8fc365

commit 5865a897f8bd64814275c2a7e90a9ad44b8fc365
Author: Demetrios Papadopoulos <dpapad@chromium.org>
Date: Thu Sep 27 21:25:54 2018

Revert "Don't propagate keydown events when dialog is open"

This reverts commit ab21d472ec0c8e9fcdc6933e485b828455011c4c.

Reason for revert: Broke
 - Print preview 'esc' behavior when dialog is open
 - Settings add-language-dialog ctrl+f behavior

Original change's description:
> Don't propagate keydown events when dialog is open
> 
> We shouldn't propagate keydown events so that it can
> behave modally.
> 
> Bug:  883221 
> Cq-Include-Trybots: luci.chromium.try:closure_compilation
> Change-Id: I4aa5257b4bf6c417123d659ba911cb01641a4cc7
> Reviewed-on: https://chromium-review.googlesource.com/1230493
> Reviewed-by: Scott Chen <scottchen@chromium.org>
> Commit-Queue: Sang Woo Ko <sangwoo108@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#593619}

TBR=calamity@chromium.org,scottchen@chromium.org,sangwoo108@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  883221 
Change-Id: I8f044d3e62ffce565a0761327b6aa877d1463224
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Reviewed-on: https://chromium-review.googlesource.com/1249809
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594871}
[modify] https://crrev.com/5865a897f8bd64814275c2a7e90a9ad44b8fc365/AUTHORS
[modify] https://crrev.com/5865a897f8bd64814275c2a7e90a9ad44b8fc365/chrome/test/data/webui/md_bookmarks/command_manager_test.js
[modify] https://crrev.com/5865a897f8bd64814275c2a7e90a9ad44b8fc365/ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js

Status: Assigned (was: Fixed)
Reverted due to regression
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7e3da6dc3d7cd7eb8a55ef301026fe1ab737f814

commit 7e3da6dc3d7cd7eb8a55ef301026fe1ab737f814
Author: sangwoo.ko <sangwoo.ko@navercorp.com>
Date: Fri Oct 05 08:49:41 2018

Introduce consume-keydown-event property to cr_dialog

For cr_dialog to behave modaaly, it's better to stop propagation of
'keydown' events. To support this, introduce consume-keydown-event
to cr_dialog. If this property is true, cr_dialog will consume
keydown events.


Bug:  883221 
Change-Id: If2981794cad08dfa6dbca8e08d2d16351fad47bd
Reviewed-on: https://chromium-review.googlesource.com/c/1254001
Commit-Queue: Sang Woo Ko <sangwoo108@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597039}
[modify] https://crrev.com/7e3da6dc3d7cd7eb8a55ef301026fe1ab737f814/AUTHORS
[modify] https://crrev.com/7e3da6dc3d7cd7eb8a55ef301026fe1ab737f814/chrome/browser/resources/md_history/history_list.html
[modify] https://crrev.com/7e3da6dc3d7cd7eb8a55ef301026fe1ab737f814/chrome/test/data/webui/cr_elements/cr_dialog_test.js
[modify] https://crrev.com/7e3da6dc3d7cd7eb8a55ef301026fe1ab737f814/ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js

Status: Fixed (was: Assigned)

Sign in to add a comment