New issue
Advanced search Search tips

Issue 848030 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Omnibox focus ring does not move with the browser window

Project Member Reported by xiy...@chromium.org, May 30 2018

Issue description

M68 and ToT (r562634)

Repro:
1. Trigger the focus ring to show up
2. Drag the browser window

Omnibox focus ring stays while the browser window is dragged to a new location. See attached screenshot.

Think we should get it fixed for M68.
 
omnibox_focus_ring.png
1.1 MB View Download
We are reverting the elevated popup with no results right now: https://chromium-review.googlesource.com/c/chromium/src/+/1081074

I don't think we need to merge the revert back to M68, since it requires custom flags to trigger this misbehavior.

Thanks.
I don't think I have any flags enabled. It seems that the feature is active on a ChromeOS device with touch by default, where MaterialDesignController gets default mode of MATERIAL_TOUCH_OPTIMIZED [1]

[1] https://cs.chromium.org/chromium/src/ui/base/material_design/material_design_controller.cc?rcl=a7394ffc28a861c88481080381845025beeac7a0&l=44
Status: Fixed (was: Assigned)
Can you still reproduce this in Canary? We should have removed this ring entirely in this CL: https://chromium.googlesource.com/chromium/src/+/c9c838db3d55e12815963062e220fd097024cdbc
It does not repro on my ToT (r564116). 

But still exist on latest M68 build (68.0.3440.15). The revert should be merged to M68.
68.0.3440.15 screenshot attached.
omnibox_focus_ring.png
879 KB View Download
Labels: Merge-Request-68
Status: Started (was: Fixed)
Okay I've requested a merge of that revert to M68. Thanks all the details here!
Project Member

Comment 7 by sheriffbot@chromium.org, Jun 6 2018

Labels: -Merge-Request-68 Hotlist-Merge-Approved Merge-Approved-68
Your change meets the bar and is auto-approved for M68. Please go ahead and merge the CL to branch 3440 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 7 2018

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

commit 743d64fec4ed566ffce6d5b947fa3d1e784b8638
Author: Tommy C. Li <tommycli@chromium.org>
Date: Thu Jun 07 00:09:08 2018

[Merge to 68] Revert "Omnibox UI Refresh: Implement Elevated Popup for Clobber State"

This reverts commit f7d36327724e88b545db24c0e7002407d77a8b0f.

This is a manually generated revert, as the revert that we committed on master has
some extra changes to accommodate merge conflicts from further code changes.

Since on the M68 branch, we don't have those further code changes, trying to cherry
pick the revert from master generated further merge conflicts.

The best solution was to make a direct revert on the M68 branch. I wish we had
trybots here, but at least we have builders.

TBR=

Bug:  848030 
Change-Id: I0d34dd611f23cd1555295860e16b6a2b8b1591f8
Reviewed-on: https://chromium-review.googlesource.com/1089752
Reviewed-by: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#226}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/chrome/browser/extensions/api/omnibox/omnibox_api_interactive_test.cc
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac_unittest.mm
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/chrome/browser/ui/search/search_tab_helper.cc
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/chrome/browser/ui/views/omnibox/omnibox_view_views_browsertest.cc
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/components/omnibox/browser/omnibox_controller.cc
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/components/omnibox/browser/omnibox_edit_model.cc
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/components/omnibox/browser/omnibox_edit_model.h
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/components/omnibox/browser/omnibox_edit_model_unittest.cc
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/components/omnibox/browser/omnibox_popup_model.cc
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/components/omnibox/browser/omnibox_popup_model.h
[modify] https://crrev.com/743d64fec4ed566ffce6d5b947fa3d1e784b8638/components/omnibox/browser/omnibox_popup_view.h

Status: Fixed (was: Started)
Cc: tommycli@chromium.org
 Issue 855665  has been merged into this issue.

Sign in to add a comment