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

Issue 838856 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

Regression : Blue focus highlight on bookmarks bar is seen misplaced.

Reported by pranjali...@etouch.net, May 2 2018

Issue description

Chrome version : 68.0.3417.0 (Official Build) dd2d3effe08392fcd5277bf052c2d3ebbeaf8cd1-refs/heads/master@{#555217}(64 bit)

OS : Mac(10.12.6,10.13.1,10.13.5)

Steps to reproduce:
1. Launch chrome and open NTP.
2. Press Tab key until focus move to ‘Apps’ shortcut, observe the focus.

Actual : Blue focus highlight on bookmark bar is seen misplaced.
Expected : Blue focus highlight on bookmark bar should be seen properly.

This is a regression issue, broken in M-68 and will below is the bisect using old script:
Good Build : 68.0.3415.0(Revision:554688) 
Bad Build : 68.0.3416.0(Revision:554961)

Unable to provide the bisect using per-revision script as Runtime Error (i.e We don't have enough builds to bisect. revlist: [] )while doing the bisect. 
Hence providing the bisect using old bisect script.

Narrow Bisect info : 
https://chromium.googlesource.com/chromium/src/+log/52134e2eadd49041f2087cc53f9cfb807ecdc7e9..9aed4f3692399b4ad59ae472367105db72e83896?pretty=fuller&n=10000
Suspecting: r554959

@Peter Boström: Could you please help to reassign if your change is not the cause for this change.

Note : This is Mac specific issue and not seen on Windows(7,8,8.1,10) and Linux(14.04 LTS) OS.

Thank you.

 
Actual_result.png
877 KB View Download
Expected_result.png
909 KB View Download
Cc: ellyjo...@chromium.org

Comment 2 by pbos@chromium.org, May 2 2018

Cc: pbos@chromium.org
Owner: weili@chromium.org
crrev.com/c/1036608 increases the bookmarks bar height to 32dp which works well in views version. weili@ is looking at another regression because of it in  issue 838630 .

Comment 3 by weili@chromium.org, May 3 2018

Status: Started (was: Assigned)
Agreed, this and 838630 should share the same root cause. I am working on the fix now.
Project Member

Comment 4 by bugdroid1@chromium.org, May 3 2018

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

commit c29efd57272886b45e3becd47976e31d9228d0a2
Author: Wei Li <weili@chromium.org>
Date: Thu May 03 16:32:09 2018

Create and use Cocoa specific layout constants

As Views based UI is undergoing new changes, that applies to MacViews
browser as well, but diverges from Cocoa based browser. This CL solves
that problem by creating a function for Cocoa browser only. This
function can get Cocoa specific constants which won't be affected by
Views side of changes.

BUG= 838630 , 838856 
TEST=Pls make sure bookmark bar, icons and menus look ok for both Cocoa
version and MacViews version on Mac. They should be same as before for
other platforms.

Change-Id: I73152c05cfac77eb6b24a4d6204acb78767c833d
Reviewed-on: https://chromium-review.googlesource.com/1041141
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Wei Li <weili@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555762}
[modify] https://crrev.com/c29efd57272886b45e3becd47976e31d9228d0a2/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm
[modify] https://crrev.com/c29efd57272886b45e3becd47976e31d9228d0a2/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller_unittest.mm
[modify] https://crrev.com/c29efd57272886b45e3becd47976e31d9228d0a2/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller_unittest.mm
[modify] https://crrev.com/c29efd57272886b45e3becd47976e31d9228d0a2/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm
[modify] https://crrev.com/c29efd57272886b45e3becd47976e31d9228d0a2/chrome/browser/ui/cocoa/browser_window_cocoa.mm
[modify] https://crrev.com/c29efd57272886b45e3becd47976e31d9228d0a2/chrome/browser/ui/layout_constants.cc
[modify] https://crrev.com/c29efd57272886b45e3becd47976e31d9228d0a2/chrome/browser/ui/layout_constants.h

Comment 5 by weili@chromium.org, May 3 2018

Status: Fixed (was: Started)

Sign in to add a comment