New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 596045 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression : In Chrome Task Manager, focus does not move to End Process button after pressing Tab key.

Reported by yfulgaon...@etouch.net, Mar 18 2016

Issue description

Chrome version : 51.0.2682.0 153a9028e5cb73e3c707b4884550499e2a08af5c-refs/heads/master@{#381839} 32/64 bit
OS : Windows (Win 7 aero enabled), Linux

Steps : 
1. Launch Chrome, open NTP and press Shift + Esc key to open the Chrome Task Manager.
2. Click on any process and press Tab key, observe

Actual : After pressing Tab key, focus does not move to End Process button.
Expected : The focus should move to End Process button after pressing Tab key.

This is a regression issue broken in 'M-50' and below is the manual regression and narrow bisect info.
Good build : 50.0.2656.0
Bad build : 50.0.2658.0

Narrow Bisect :
https://chromium.googlesource.com/chromium/src/+log/b48692ea0b3340d51fb95fa500b176f819623ee1..2b4fc356ff151e0c104596bffea377d6fad40281?pretty=fuller&n=10000

Suspecting : r376888 from Narrow Bisect

Note : Above issue is not seen on Mac OS.



 
Actual_task manager_focus.mp4
584 KB Download
Expected_task manager_focus.mp4
701 KB Download
Labels: ReleaseBlock-Stable
Adding release block label, please undo if not the case.
Just to update, issue is still observed on chrome version 51.0.2686.0 on Windows and Ubuntu 14.04.

@karandeepb: request you to please take a look into it.
Labels: -M-50 M-51
I have confirmed that the issue is due to my CL. Will be working on a fix. However, I think its too late to merge to M50. Changing to M51.
Status: Started (was: Assigned)
Cc: ashej...@chromium.org
@karandeepb: Hey, would you mind providing an update on the above issue as it is reproducible on Ubuntu 14.04 & Windows with chrome version '51.0.2693.2'.

Appreciate your response.

Thank you!
@ashejole: I had put up a CL for this which was under review - https://codereview.chromium.org/1826433002/. However, I am OOO this week. You can expect the fix to land next week.
@karandeep: Thanks for the update, much appreciated.
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 11 2016

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

commit e5b001fd75dc0fc7dbe265020ffbd84be9916d5c
Author: karandeepb <karandeepb@chromium.org>
Date: Mon Apr 11 01:21:46 2016

DialogClientView: Fix regression in Chrome Task Manager focusing.

This CL fixes a regression introduced in
https://codereview.chromium.org/1690133003/. The regression is caused since the
TaskManagerView, adds child views to the DialogClientView. However,
DialogClientView::SetupFocusChain does not account for externally added child
views.

This CL uses View::ReorderChildView to reorder child views and ensure that
appropriate focus order is maintained. Also the redundant contents_ view in
DialogClientViewTest is removed.

BUG= 596045 ,  586419 

Review URL: https://codereview.chromium.org/1826433002

Cr-Commit-Position: refs/heads/master@{#386324}

[modify] https://crrev.com/e5b001fd75dc0fc7dbe265020ffbd84be9916d5c/ui/views/window/dialog_client_view.cc
[modify] https://crrev.com/e5b001fd75dc0fc7dbe265020ffbd84be9916d5c/ui/views/window/dialog_client_view_unittest.cc

Labels: Merge-Request-51
Status: Fixed (was: Started)
Have verified this is Windows Canary.

Comment 10 by tin...@google.com, Apr 13 2016

Labels: -Merge-Request-51 Merge-Approved-51 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M51 (branch: 2704)
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 13 2016

Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/026a1371d6544af1856aa83e3077139a7c2aae39

commit 026a1371d6544af1856aa83e3077139a7c2aae39
Author: Trent Apted <tapted@chromium.org>
Date: Wed Apr 13 00:34:57 2016

[merge] DialogClientView: Fix regression in Chrome Task Manager focusing.

This CL fixes a regression introduced in
https://codereview.chromium.org/1690133003/. The regression is caused since the
TaskManagerView, adds child views to the DialogClientView. However,
DialogClientView::SetupFocusChain does not account for externally added child
views.

This CL uses View::ReorderChildView to reorder child views and ensure that
appropriate focus order is maintained. Also the redundant contents_ view in
DialogClientViewTest is removed.

BUG= 596045 ,  586419 
TBR=karandeepb@chromium.org

Review URL: https://codereview.chromium.org/1826433002

Cr-Commit-Position: refs/heads/master@{#386324}
(cherry picked from commit e5b001fd75dc0fc7dbe265020ffbd84be9916d5c)

Review URL: https://codereview.chromium.org/1879273002 .

Cr-Commit-Position: refs/branch-heads/2704@{#20}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/026a1371d6544af1856aa83e3077139a7c2aae39/ui/views/window/dialog_client_view.cc
[modify] https://crrev.com/026a1371d6544af1856aa83e3077139a7c2aae39/ui/views/window/dialog_client_view_unittest.cc

Sign in to add a comment