New issue
Advanced search Search tips

Issue 848641 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Regression : Blue focus ring over omnibox is not seen after pressing 'F6' key from keyboard.

Reported by rp...@etouch.net, Jun 1 2018

Issue description

Chrome version: 69.0.3447.0 (Official Build) Revision ec57e30ccad35465163bffcc3860ac8b30a6fb33-refs/branch-heads/3447@{#1}(32/64-bit)
OS: Windows (7,8,8.1,10),Linux (14.04 LTS)

What steps will reproduce the problem?
1. Launch chrome,press 'F6' key from keyboard and observe blue focus ring over omnibox
 
Actual: Blue focus ring over omnibox is not seen after pressing 'F6' key
Expected: Blue focus ring over omnibox should be seen after pressing 'F6' key

This is regression issue, broken in ‘M 69’ and will soon update other info :
Good build: 69.0.3445.0  (Revision: 562691).
Bad build: 69.0.3446.0 (Revision: 563011).

 
Actual_video.mp4
279 KB View Download
Expected_video.mp4
172 KB View Download
Actual_screenshot.png
105 KB View Download
Expected_screenshot.png
105 KB View Download

Comment 1 by rp...@etouch.net, Jun 1 2018

Labels: hasbisect
Owner: tommycli@chromium.org
Status: Assigned (was: Unconfirmed)
You are probably looking for a change made after 562969 (known good), but no later than 562979 (first known bad).

Narrow Bisect info : 
https://chromium.googlesource.com/chromium/src/+log/5ecb1c29f9cd7fa027d0c573286f78a55da16188..be652b796af75662f2d0dc64e152d8c90b14f271?pretty=fuller&n=50

Suspecting: r562979 from Narrow bisect

@tommycli: Could you please help to reassign if your change is not the cause for this change.

Note:
1.Unable to provide bisect using per-revision script,Hence providing bisect with old script.
2.Pardon me if it is an intended change then Blue focus highlight also not seen over omnibox even when it is focused.
3.Issue is not seen on Mac(10.12.6,10.13.1,10.13.6) 




Project Member

Comment 2 by bugdroid1@chromium.org, Jun 1 2018

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

commit 479e0af586ef7a0b0770312a9f03a13aa72910c2
Author: Tommy C. Li <tommycli@chromium.org>
Date: Fri Jun 01 21:50:31 2018

Omnibox UI Refresh: Fix legacy focus ring for keyboard accessibility

Previously we displayed a legacy focus ring if Material Refresh was off
for full keyboard accessibilty mode on all platforms.

The CL below accidentally removed it:
https://chromium-review.googlesource.com/c/chromium/src/+/1069690

Now we are adding it back with a partial revert - and a refactor to
make the logic overall more clear. We can remove everything we are
adding in this CL once Material Refresh is permanent and launched.

Bug:  848641 ,  844263 ,  823535 
Change-Id: Ife78cd9660e38f16525b55044ee0a231101534a6
Reviewed-on: https://chromium-review.googlesource.com/1082794
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563814}
[modify] https://crrev.com/479e0af586ef7a0b0770312a9f03a13aa72910c2/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/479e0af586ef7a0b0770312a9f03a13aa72910c2/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/479e0af586ef7a0b0770312a9f03a13aa72910c2/chrome/browser/ui/views/toolbar/toolbar_view.cc
[modify] https://crrev.com/479e0af586ef7a0b0770312a9f03a13aa72910c2/chrome/browser/ui/views/toolbar/toolbar_view.h

Status: Fixed (was: Assigned)

Comment 4 by rp...@etouch.net, Jun 6 2018

Labels: TE-Verified-69.0.3451.0 TE-Verified-M69
Update : 
Rechecked this issue on Windows (7,8,8.1,10) and Linux (14.04 LTS) OS with latest Canary Chrome version: 69.0.3451.0 and the issue is fixed.Kindly refer attached screen cast for your reference.
Fixed_video.mp4
320 KB View Download
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 26 2018

Labels: merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e41d3cc0b23587b7dde5ebad20a69227b341de28

commit e41d3cc0b23587b7dde5ebad20a69227b341de28
Author: Tommy C. Li <tommycli@chromium.org>
Date: Tue Jun 26 23:56:49 2018

[Merge to M68] Omnibox UI Refresh: Always show the focus ring when focused

This merge comprises of the following two CLs together, since the
intermediate state is broken:

Omnibox UI Refresh: Always show the focus ring when focused

Previously, we only showed the focus ring on certain platforms (Mac) or
keyboard accessibility mode.

Now, following the new Material Refresh guidelines, we display the focus
ring on the Omnibox whenever it is focused (and the popup is hidden).

This also simplifies the code a bit, as keyboard accessibility mode is
no longer relevant.

Bug:  844263 ,  823535 
Change-Id: Ic5133463f6e67af5ef6f0512034938e4a19a9972
Reviewed-on: https://chromium-review.googlesource.com/1069690
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562979}

(cherry picked from commit be652b796af75662f2d0dc64e152d8c90b14f271)

Omnibox UI Refresh: Fix legacy focus ring for keyboard accessibility

Previously we displayed a legacy focus ring if Material Refresh was off
for full keyboard accessibilty mode on all platforms.

The CL below accidentally removed it:
https://chromium-review.googlesource.com/c/chromium/src/+/1069690

Now we are adding it back with a partial revert - and a refactor to
make the logic overall more clear. We can remove everything we are
adding in this CL once Material Refresh is permanent and launched.

(cherry picked from commit 479e0af586ef7a0b0770312a9f03a13aa72910c2)

Bug:  848641 ,  844263 ,  823535 
Change-Id: Ife78cd9660e38f16525b55044ee0a231101534a6
Reviewed-on: https://chromium-review.googlesource.com/1082794
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#563814}
Reviewed-on: https://chromium-review.googlesource.com/1115432
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#543}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/e41d3cc0b23587b7dde5ebad20a69227b341de28/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/e41d3cc0b23587b7dde5ebad20a69227b341de28/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/e41d3cc0b23587b7dde5ebad20a69227b341de28/chrome/browser/ui/views/toolbar/toolbar_view.cc
[modify] https://crrev.com/e41d3cc0b23587b7dde5ebad20a69227b341de28/ui/views/controls/focus_ring.cc
[modify] https://crrev.com/e41d3cc0b23587b7dde5ebad20a69227b341de28/ui/views/controls/focus_ring.h

Comment 6 by rp...@etouch.net, Jun 27 2018

Labels: TE-Verified-M68 TE-Verified-68.0.3440.42
Update : 
Rechecked this issue on Windows (7,8,8.1,10) and Linux (14.04 LTS) OS using Beta build : 68.0.3440.42 and the issue is fixed.Hence adding TE-Verified Labels.

Kindly refer attached screen cast for your reference. 
Fixed_behavior.mp4
226 KB View Download

Sign in to add a comment