New issue
Advanced search Search tips

Issue 808273 link

Starred by 1 user
Status: Fixed
Owner:
pbos@chromium.org
Closed: Feb 2018
Cc:
bsep@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

BubbleIconViews' inkdrop not being highlighted automatically when a corresponding bubble spawns.

Project Member Reported by pbos@chromium.org, Feb 2 2018 
StarViews and ManagePasswordsIconViews (land in progress) both contain code to highlight that is manually triggered by the code path spawning the dialog. This is error prone as shown by Translate currently not highlighting their corresponding icon.

Some highlighting code should probably go into BubbleIconView, and the static spawn function should not have to call it manually.
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 20 2018 

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

commit e577c468e4149b07f2abae9058147d4464e521d1
Author: Peter Boström <pbos@chromium.org>
Date: Tue Feb 20 23:25:57 2018

Highlight all BubbleIconView instances.

Highlights the BubbleIconView whenever the corresponding widget is
visible. This was previously a one-off done for the bookmark star view.

This addresses highlighting the translate icon, bookmark icon, zoom icon
and others in a single place. This makes it less error prone and more
future proof.

BubbleIconView still has to be added as an observer of the bubble widget
(but this is already required to remove button-press highlights when the
widget closes).

Bug:  chromium:808273 
Change-Id: Ibd4e14b4876be3e79ef8a85a0a9feade65aa5628
Reviewed-on: https://chromium-review.googlesource.com/905408
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537928}
[modify] https://crrev.com/e577c468e4149b07f2abae9058147d4464e521d1/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/e577c468e4149b07f2abae9058147d4464e521d1/chrome/browser/ui/views/location_bar/bubble_icon_view.cc
[modify] https://crrev.com/e577c468e4149b07f2abae9058147d4464e521d1/chrome/browser/ui/views/location_bar/bubble_icon_view.h
[modify] https://crrev.com/e577c468e4149b07f2abae9058147d4464e521d1/chrome/browser/ui/views/location_bar/star_view.cc
[modify] https://crrev.com/e577c468e4149b07f2abae9058147d4464e521d1/chrome/browser/ui/views/location_bar/star_view.h
[modify] https://crrev.com/e577c468e4149b07f2abae9058147d4464e521d1/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc
[modify] https://crrev.com/e577c468e4149b07f2abae9058147d4464e521d1/chrome/browser/ui/views/passwords/manage_passwords_icon_views.cc
[modify] https://crrev.com/e577c468e4149b07f2abae9058147d4464e521d1/chrome/browser/ui/views/passwords/manage_passwords_icon_views.h
[modify] https://crrev.com/e577c468e4149b07f2abae9058147d4464e521d1/chrome/browser/ui/views/passwords/password_bubble_view_base.cc
[modify] https://crrev.com/e577c468e4149b07f2abae9058147d4464e521d1/chrome/browser/ui/views/toolbar/toolbar_view.cc

Comment 2 by pbos@chromium.org, Feb 20 2018

Status: Fixed (was: Available)
►

Sign in to add a comment