New issue
Advanced search Search tips

Issue 910067 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 18
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression:Focus ring is not seen for 'Reload' and 'X' buttons in 'Reload' bar.

Reported by shruti.j...@etouch.net, Nov 29

Issue description

Chrome Version: 72.0.3625.0 (Official Build) Revision 2c59a8a07afb8b11354406e63270d1cbeb582c47-refs/branch-heads/3625@{#1}(32/64 bit)
OS : Mac(10.13.6, 10.13.1, 10.14.2)

Test URL:https://permission.site/

Steps to reproduce:
1. Launch chrome and navigate to above URL.
2. Click on any permission Eg..'Notification' and permission bubble pop ups.
3. Click on Allow/Block and change the permission of 'Notification' by clicking on secure chip.
4. Reload Bar appears ,Press Tab key to bring focus on 'Reload' and 'X' buttons.
5. Observe

Actual Result   : Focus ring  is not seen for 'Reload' and 'X' buttons in 'Reload' bar.
Expected Result : Focus ring should be seen  for 'Reload' and 'X' buttons in 'Reload' bar.

This is a regression issue broken in M-71 and below is the bisect information:
Good Build : 71.0.3554.0(Revision : 591605)
Bad Build :  71.0.3555.0(Revision : 591860)

You are probably looking for a change made after 591811 (known good), but no later than 591812 (first known bad).
CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.

https://chromium.googlesource.com/chromium/src/+log/4b33833b417cdfc6885390f3c5145ff8456ffbac..ad06a605ed9819946c82eaff760c4406387fe240


Suspecting: https://chromium.googlesource.com/chromium/src/+/ad06a605ed9819946c82eaff760c4406387fe240

@Dana Fried :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.	

Kindly refer the attached screencast.

Thank You!
 

 
Actual_result.mov
2.0 MB View Download
Expected_result.mov
10.7 MB View Download
Status: Started (was: Assigned)
This apparently mostly worked by accident before. I'll see what I can do to ensure focus on the reload bar.
Labels: Hotlist-DesktopUIConsider
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 17

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

commit 46c516e9ed2d91e637bf16428be9fe8a390ff60f
Author: Dana Fried <dfried@chromium.org>
Date: Mon Dec 17 23:00:52 2018

Added Widget::CloseWithReason().

In support of having smart behavior when widgets are closing re: where
the focus should go next.

Added a way for widgets that do not have a custom ClientView to bail
out of a Close[WithReason]() call; required for a specific
ChromeOS/Android use case.

Bug:  910067 
Change-Id: Icc34f2a0dea696cb30f54248004e7c1c9998f75a
Reviewed-on: https://chromium-review.googlesource.com/c/1371064
Commit-Queue: Dana Fried <dfried@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617275}
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_container.cc
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/chrome/browser/ui/views/constrained_web_dialog_delegate_views.cc
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/chrome/browser/ui/views/translate/translate_bubble_view.cc
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/components/exo/shell_surface_base.cc
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/components/exo/shell_surface_base.h
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/ui/views/bubble/bubble_dialog_delegate_view.cc
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/ui/views/bubble/bubble_frame_view.cc
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/ui/views/bubble/bubble_frame_view.h
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/ui/views/test/widget_test.cc
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/ui/views/test/widget_test.h
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/ui/views/widget/widget.cc
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/ui/views/widget/widget.h
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/ui/views/widget/widget_delegate.cc
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/ui/views/widget/widget_delegate.h
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/ui/views/widget/widget_unittest.cc
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/ui/views/window/custom_frame_view.cc
[modify] https://crrev.com/46c516e9ed2d91e637bf16428be9fe8a390ff60f/ui/views/window/dialog_client_view.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 17

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

commit 99115b5f73e233211078be11a9bff8a756750ce2
Author: Dana Fried <dfried@chromium.org>
Date: Mon Dec 17 23:46:45 2018

Fix focusability and tab order of infobar.

Ensures that despite odd ordering of browserview's children for painting
purposes, tab order is as expected.

Also ensures that focus returns to the omnibox following the user
interacting with the security chip, so that the next item in tab order
is the reload prompt.

Setting focus directly to the infobar would be very tricky and require
additional architectural changes, since the infobar is not visible (and
therefore not focusable) at the moment the prompt is created and
installed.

Bug:  910067 
Change-Id: Ia956c60401696608c3307620f58b15a0a428d136
Reviewed-on: https://chromium-review.googlesource.com/c/1362499
Commit-Queue: Dana Fried <dfried@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617290}
[modify] https://crrev.com/99115b5f73e233211078be11a9bff8a756750ce2/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/99115b5f73e233211078be11a9bff8a756750ce2/chrome/browser/ui/views/frame/browser_view.h
[modify] https://crrev.com/99115b5f73e233211078be11a9bff8a756750ce2/chrome/browser/ui/views/page_info/page_info_bubble_view.cc

Status: Fixed (was: Started)

Sign in to add a comment