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

Issue 788051 link

Starred by 2 users
Status: Started
Owner:
alancutter@chromium.org
Cc:
█ mgiuca@chromium.org
calamity@chromium.org
█ austinknight@chromium.org
jopra@chromium.org
dominickn@chromium.org
█ bgalbs@google.com
pcovell@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 2
Type: Feature

Blocked on:
issue 828278


Sign in to add a comment

Desktop PWAs: Show other Omnibox UI features in title bar

Project Member Reported by mgiuca@chromium.org, Nov 23 2017 
Features like password manager, payments, protocol handlers, etc, show UI in the Omnibox. For installed DPWAs, we should probably surface this UI in the title bar.

Right now, the UI associated with these icons does work (e.g., if you are offered to save a password, the bubble appears), but it is not anchored to an icon, and there is no way to click it if you close the bubble.

Something for after V1, since it is not critical.

Comment 1 by mgiuca@chromium.org, Feb 13 2018

Cc: mgiuca@chromium.org
Owner: jopra@chromium.org
Status: Started (was: Untriaged)

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

This is a dup/super set of https://bugs.chromium.org/p/chromium/issues/detail?id=800210

Comment 3 by jopra@chromium.org, Feb 20 2018 

 Issue 800210  has been merged into this issue.
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 9 2018 

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

commit 1e358c58a523fe65d14ccb8d6f9e653dcbcfd159
Author: Josh Pratt <jopra@google.com>
Date: Fri Mar 09 05:53:26 2018

Refactor LocationBarView to move Refresh logic for some BubbleIconViews into the respective subclasses.

This is initial work for adding BubbleIconViews to Chrome OS Desktop PWAs.
It partially decouples BubbleIconViews from the LocationBarView so that
they can also be used in HostedAppButtonContainer.

Bug: 788051
Change-Id: I986951b64fb02f2d2728abbe6a3964ff97e04909
Reviewed-on: https://chromium-review.googlesource.com/923607
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542045}
[modify] https://crrev.com/1e358c58a523fe65d14ccb8d6f9e653dcbcfd159/chrome/browser/ui/views/autofill/save_card_icon_view.cc
[modify] https://crrev.com/1e358c58a523fe65d14ccb8d6f9e653dcbcfd159/chrome/browser/ui/views/autofill/save_card_icon_view.h
[modify] https://crrev.com/1e358c58a523fe65d14ccb8d6f9e653dcbcfd159/chrome/browser/ui/views/location_bar/bubble_icon_view.cc
[modify] https://crrev.com/1e358c58a523fe65d14ccb8d6f9e653dcbcfd159/chrome/browser/ui/views/location_bar/bubble_icon_view.h
[modify] https://crrev.com/1e358c58a523fe65d14ccb8d6f9e653dcbcfd159/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/1e358c58a523fe65d14ccb8d6f9e653dcbcfd159/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/1e358c58a523fe65d14ccb8d6f9e653dcbcfd159/chrome/browser/ui/views/passwords/manage_passwords_icon_views.cc
[modify] https://crrev.com/1e358c58a523fe65d14ccb8d6f9e653dcbcfd159/chrome/browser/ui/views/passwords/manage_passwords_icon_views.h
[modify] https://crrev.com/1e358c58a523fe65d14ccb8d6f9e653dcbcfd159/chrome/browser/ui/views/translate/translate_icon_view.cc
[modify] https://crrev.com/1e358c58a523fe65d14ccb8d6f9e653dcbcfd159/chrome/browser/ui/views/translate/translate_icon_view.h

Comment 5 by mgiuca@chromium.org, Mar 26 2018

Cc: jopra@chromium.org
Owner: alancutter@chromium.org
Reassigning to Alan. (I think Josh has transferred the CL to you, is that correct?)

Comment 6 by mgiuca@chromium.org, Mar 26 2018

Labels: M-67

Comment 8 by alancutter@chromium.org, Mar 27 2018 

@mgiuca should this be Pri 3?

Comment 9 by mgiuca@chromium.org, Mar 27 2018

Labels: -Pri-3 Pri-2
No, we should have this in. Pri-2. Is it feasible?

Comment 10 by alancutter@chromium.org, Mar 29 2018 

This is blocked on refactoring our toolbar logic to make sure uses of buttons in normally in the location bar don't assume they're in the location bar.
WIP notes: https://docs.google.com/spreadsheets/d/1TdsGZHSte18gSF9o69dcYtkJZdITxeSvoTXs-I1hvB8/edit#gid=0

Comment 11 by alancutter@chromium.org, Apr 3 2018

Blockedon: 828278

Comment 12 by alancutter@chromium.org, Apr 13 2018

Labels: -M-67 M-68
67 has branched, moving bugs over to 68.
Project Member

Comment 13 by bugdroid1@chromium.org, May 14 2018 

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

commit 2cce5382f0df68d9f8c19a3660a920917643e458
Author: Alan Cutter <alancutter@chromium.org>
Date: Mon May 14 18:20:48 2018

Add PageActionIconContainer interface

This CL is the start of a refactor to pull update methods for
page action icons out of the LocationBar interface and into
a PageActionIconContainer interface so that page action icons
can live outside of the location bar.

Design doc: https://docs.google.com/document/d/18HJywSmw6JiYMxFTbP496ivilPblOT8A63V12ip3ymE/

Bug: 788051
Change-Id: I2ae0e289d1e777e7bde5d0d73094dbbc60b2109a
Reviewed-on: https://chromium-review.googlesource.com/1049126
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558378}
[modify] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/browser_window.h
[modify] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/cocoa/browser_window_cocoa.h
[modify] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/cocoa/browser_window_cocoa.mm
[modify] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h
[modify] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm
[add] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/page_action/page_action_icon_container.h
[modify] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/views/frame/browser_view.h
[modify] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/views/frame/hosted_app_button_container.cc
[modify] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/views/frame/hosted_app_button_container.h
[modify] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/views/frame/toolbar_button_provider.h
[modify] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/views/location_bar/location_bar_view.h
[add] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/views/page_action/page_action_icon_container_view.cc
[add] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/views/page_action/page_action_icon_container_view.h
[modify] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/views/toolbar/toolbar_view.cc
[modify] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/browser/ui/views/toolbar/toolbar_view.h
[modify] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/test/base/test_browser_window.cc
[modify] https://crrev.com/2cce5382f0df68d9f8c19a3660a920917643e458/chrome/test/base/test_browser_window.h
Project Member

Comment 14 by bugdroid1@chromium.org, May 16 2018 

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

commit d3106d2b65ecc268873f2a0c0cc92586e8553a4f
Author: Alan Cutter <alancutter@chromium.org>
Date: Wed May 16 05:31:00 2018

Rename BubbleIconView to PageActionIconView

This is a non-behaviour changing class rename.
BubbleIconView is only used for page action icons.
This CL renames BubbleIconView to PageActionIconView to
befit the PageActionIconContainerView class.

This is part of a refactor to allow page action icons to
appear outside of the location bar and begins the migration
of these icons from chrome/browser/ui/views/location_bar
to chrome/browser/ui/views/page_action.

Relevant design doc:
https://docs.google.com/document/d/18HJywSmw6JiYMxFTbP496ivilPblOT8A63V12ip3ymE/

Bug: 788051
Change-Id: Ic3bb1d3a17a2999fc277e3c05b5523ea19be8441
Reviewed-on: https://chromium-review.googlesource.com/1056783
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558987}
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/autofill/save_card_icon_view.cc
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/autofill/save_card_icon_view.h
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/location_bar/find_bar_icon.cc
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/location_bar/find_bar_icon.h
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/location_bar/intent_picker_view.cc
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/location_bar/intent_picker_view.h
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/location_bar/location_icon_view_interactive_uitest.cc
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/location_bar/star_view.cc
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/location_bar/star_view.h
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/location_bar/zoom_view.cc
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/location_bar/zoom_view.h
[rename] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/page_action/page_action_icon_view.cc
[rename] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/page_action/page_action_icon_view.h
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/passwords/manage_passwords_icon_views.cc
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/passwords/manage_passwords_icon_views.h
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/toolbar/toolbar_view.cc
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/translate/translate_icon_view.cc
[modify] https://crrev.com/d3106d2b65ecc268873f2a0c0cc92586e8553a4f/chrome/browser/ui/views/translate/translate_icon_view.h
Project Member

Comment 15 by bugdroid1@chromium.org, May 16 2018 

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

commit 223e651d3ba5bef5290490c129eaf2da1a2c0695
Author: Alan Cutter <alancutter@chromium.org>
Date: Wed May 16 06:54:09 2018

Add OWNERS file to chrome/browser/ui/views/page_action/

This CL adds alancutter and calamity as owners for the newly created
chrome/browser/ui/views/page_action/ directory.
alancutter is working to migrate page action icons in the location bar
to be more general purpose. calamity has more experience of views paradigms
and has been collaborating on the refactor design.

Further context for this directory:
https://chromium-review.googlesource.com/c/chromium/src/+/1056783

Bug: 788051
Change-Id: I9b3150a4426b659105217ef14716f8671266ef93
Reviewed-on: https://chromium-review.googlesource.com/1060775
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559000}
[add] https://crrev.com/223e651d3ba5bef5290490c129eaf2da1a2c0695/chrome/browser/ui/views/page_action/OWNERS

Comment 17 by alancutter@chromium.org, May 17 2018 

Some screenshots of WIP page action icon colours.
zoom-icon-native-theme.png
7.8 KB View Download
zoom-icon-light-theme.png
7.6 KB View Download
zoom-icon-dark-theme.png
7.0 KB View Download
Project Member

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

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

commit 7edcaa69bd673a05d58b958948b3f80d765e552f
Author: Alan Cutter <alancutter@chromium.org>
Date: Thu Jun 07 01:29:53 2018

Rename PageActionIconView::Refresh() to Update()

This CL renames PageActionIconView::Refresh() to be Update().
This makes it more consistent with
PageActionIconContainer::UpdatePageActionIcon() and LocationBar methods
like UpdateSaveCreditCardIcon().

There are no behavioural changes in this CL.

Bug: 788051
Change-Id: I1d5d05cfc659d89a41480771e2415b261553263b
Reviewed-on: https://chromium-review.googlesource.com/1082064
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565135}
[modify] https://crrev.com/7edcaa69bd673a05d58b958948b3f80d765e552f/chrome/browser/ui/views/autofill/save_card_icon_view.cc
[modify] https://crrev.com/7edcaa69bd673a05d58b958948b3f80d765e552f/chrome/browser/ui/views/autofill/save_card_icon_view.h
[modify] https://crrev.com/7edcaa69bd673a05d58b958948b3f80d765e552f/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/7edcaa69bd673a05d58b958948b3f80d765e552f/chrome/browser/ui/views/location_bar/zoom_view.cc
[modify] https://crrev.com/7edcaa69bd673a05d58b958948b3f80d765e552f/chrome/browser/ui/views/location_bar/zoom_view.h
[modify] https://crrev.com/7edcaa69bd673a05d58b958948b3f80d765e552f/chrome/browser/ui/views/page_action/page_action_icon_view.cc
[modify] https://crrev.com/7edcaa69bd673a05d58b958948b3f80d765e552f/chrome/browser/ui/views/page_action/page_action_icon_view.h
[modify] https://crrev.com/7edcaa69bd673a05d58b958948b3f80d765e552f/chrome/browser/ui/views/passwords/manage_passwords_icon_views.cc
[modify] https://crrev.com/7edcaa69bd673a05d58b958948b3f80d765e552f/chrome/browser/ui/views/passwords/manage_passwords_icon_views.h
[modify] https://crrev.com/7edcaa69bd673a05d58b958948b3f80d765e552f/chrome/browser/ui/views/translate/translate_icon_view.cc
[modify] https://crrev.com/7edcaa69bd673a05d58b958948b3f80d765e552f/chrome/browser/ui/views/translate/translate_icon_view.h
Project Member

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

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

commit 9b87a12fabc93c8dda609b4c00dd0a6b029baeca
Author: Alan Cutter <alancutter@chromium.org>
Date: Thu Jun 07 01:36:35 2018

Rename PageActionIconView::UpdateIcon() to UpdateIconImage()

This rename helps disambiguate Update() and UpdateIcon() by renaming the
latter as UpdateIconImage().
The former updates the view's visibility state while the latter updates
the vector icon used by painting.

Bug: 788051
Change-Id: I1a6d24ed404b5a2017e64ea9b064ada6de7865dd
Reviewed-on: https://chromium-review.googlesource.com/1088453
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565136}
[modify] https://crrev.com/9b87a12fabc93c8dda609b4c00dd0a6b029baeca/chrome/browser/ui/views/location_bar/star_view.cc
[modify] https://crrev.com/9b87a12fabc93c8dda609b4c00dd0a6b029baeca/chrome/browser/ui/views/location_bar/zoom_view.cc
[modify] https://crrev.com/9b87a12fabc93c8dda609b4c00dd0a6b029baeca/chrome/browser/ui/views/page_action/page_action_icon_view.cc
[modify] https://crrev.com/9b87a12fabc93c8dda609b4c00dd0a6b029baeca/chrome/browser/ui/views/page_action/page_action_icon_view.h

Comment 20 by mgiuca@chromium.org, Jun 18 2018

Labels: -M-68 M-69
Bulk punting M68 PWA bugs to M69.
Project Member

Comment 21 by bugdroid1@chromium.org, Jul 3 

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

commit 108c0687254683b3beeaa32b6e0a86f2046a867e
Author: Alan Cutter <alancutter@chromium.org>
Date: Tue Jul 03 05:03:29 2018

Add the zoom icon to hosted app title bars

This CL moves the ZoomView icon from LocationBarView to
PageActionIconContainerView for reusability in the
HostedAppButtonContainer. The user visible effect of this is
the zoom icon can be seen in hosted app window title bars, see
screenshots below.

As part of this generalisation:
 - The PageActionIconView::Delegate class now reads an
   SkColor instead of an OmniboxTint for consistency with
   other title bar icons, see screenshots below.
 - The size of the zoom icon (and other page action icons that
   get migrated in the future) is reduced in touch optimised
   mode for the HostedAppButtonContainer to avoid stretching
   the title bar region vertically when shown.

Native icon colours: https://bugs.chromium.org/p/chromium/issues/attachment?aid=339750&signed_aid=0n6nlPPzzcaH-Xc58twlyA==&inline=1
Light theme icon colours: https://bugs.chromium.org/p/chromium/issues/attachment?aid=339751&signed_aid=Rg4OpeJyhj-_-5vGILuzYw==&inline=1
Dark theme icon colours: https://bugs.chromium.org/p/chromium/issues/attachment?aid=339752&signed_aid=EbybLy_ZlxoIfr2cKOjqzw==&inline=1

Bug: 788051
Change-Id: I8b761f1efbce8cdb62c6501327fdfcfe7ba7d912
Reviewed-on: https://chromium-review.googlesource.com/1051141
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572114}
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/layout_constants.cc
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/layout_constants.h
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/location_bar/location_bar.h
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/page_action/page_action_icon_container.h
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/views/frame/hosted_app_button_container.cc
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/views/frame/hosted_app_button_container.h
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/views/location_bar/location_bar_bubble_delegate_view.h
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/views/location_bar/location_bar_view_browsertest.cc
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/views/location_bar/zoom_bubble_view.h
[delete] https://crrev.com/0e41546585711ceaa448092981a8273088313d24/chrome/browser/ui/views/location_bar/zoom_view.cc
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/views/page_action/page_action_icon_container_view.cc
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/views/page_action/page_action_icon_container_view.h
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/views/page_action/page_action_icon_view.cc
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/views/page_action/page_action_icon_view.h
[add] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/views/page_action/zoom_view.cc
[rename] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/browser/ui/views/page_action/zoom_view.h
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/chrome/test/base/test_browser_window.h
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/components/zoom/zoom_event_manager.cc
[modify] https://crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e/components/zoom/zoom_event_manager.h

Comment 22 by alancutter@chromium.org, Jul 4

Cc: austinknight@chromium.org
WIP adding the find icon to the title bar.
https://chromium-review.googlesource.com/c/chromium/src/+/1054908
before.png
69.7 KB View Download
after.png
70.0 KB View Download

Comment 23 by alancutter@chromium.org, Jul 4 

Looks like the find icon got nudged to the left by r108c0687254683b3beeaa32b6e0a86f2046a867e, the WIP CL puts it back to where it should be.

Also fixed up the find bar boundary to match the omnibox in the main browser.
pre-zoom-patch.png
54.0 KB View Download
master.png
53.8 KB View Download
find-patch.png
55.2 KB View Download

Comment 24 by alancutter@chromium.org, Jul 4 

Updated the find bar to anchor to the three dot menu.
find-patch.png
55.3 KB View Download

Comment 25 by alancutter@chromium.org, Jul 5 

Updated patch to handle RTL UI better.
rtl-find-bar.png
56.1 KB View Download
Project Member

Comment 26 by bugdroid1@chromium.org, Jul 12 

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

commit e35f205a1441afebe9987e606c7c876d5bfc4594
Author: Alan Cutter <alancutter@chromium.org>
Date: Thu Jul 12 07:03:30 2018

Add find bar icon to hosted app windows

This CL moves the find icon from LocationBarView into PageActionIconContainerView
enabling the icon to appear in hosted app window title bars.

This change also fixes the find icon getting nudged by
crrev.com/108c0687254683b3beeaa32b6e0a86f2046a867e.

Before 108c0687254683b3beeaa32b6e0a86f2046a867e:
https://bugs.chromium.org/p/chromium/issues/attachment?aid=346964&signed_aid=IuPvufZswamla90tWW5FCg==&inline=1
Before CL:
https://bugs.chromium.org/p/chromium/issues/attachment?aid=346965&signed_aid=Aiy5ATHUK4cWS64MyuRMdQ==&inline=1
After CL:
https://bugs.chromium.org/p/chromium/issues/attachment?aid=346968&signed_aid=DR3vtzLR7tlPH3cEwibshQ==&inline=1

Bug: 788051, 814531
Change-Id: I5fa60cce484bd2553131582fc39e6737da34e967
Reviewed-on: https://chromium-review.googlesource.com/1054908
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574499}
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/find_bar/find_bar_controller.cc
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/location_bar/location_bar.h
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/page_action/page_action_icon_container.h
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/views/frame/browser_view_layout.cc
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/views/frame/hosted_app_button_container.cc
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/views/frame/hosted_app_button_container.h
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/views/frame/toolbar_button_provider.h
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/views/location_bar/find_bar_icon.cc
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/views/location_bar/find_bar_icon.h
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/views/page_action/page_action_icon_container_view.cc
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/views/page_action/page_action_icon_container_view.h
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/views/toolbar/toolbar_view.cc
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/browser/ui/views/toolbar/toolbar_view.h
[modify] https://crrev.com/e35f205a1441afebe9987e606c7c876d5bfc4594/chrome/test/base/test_browser_window.h

Comment 27 Deleted

Comment 28 by hwi@chromium.org, Oct 5 

Cast popover dialog should be also anchored to its own cast icon similar to Find and Zoom.

Currently it's center-aligned without an anchor icon - 71.0.3568.0 (Official Build) canary (64-bit).
cast.png
107 KB View Download
cast - browser.png
86.4 KB View Download

Comment 29 by alancutter@chromium.org, Nov 21

Labels: -M-69 OS-Linux OS-Windows
WIP CL for adding the password manager icon to the title bar:
https://chromium-review.googlesource.com/c/chromium/src/+/1345672
before-title-bar.png
15.2 KB View Download
after-title-bar.png
22.3 KB View Download
before-location-bar.png
31.5 KB View Download
after-location-bar.png
31.6 KB View Download
Project Member

Comment 30 by bugdroid1@chromium.org, Nov 28 

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

commit 33845c065ed4ae2585eeb340175d9133ecf2e8a0
Author: Alan Cutter <alancutter@chromium.org>
Date: Wed Nov 28 01:32:19 2018

Add manage passwords icon to PWA windows

This CL allows PWA title bars to show the manage passwords icon
by moving it from LocationBarView to PageActionIconContainerView.

PWA title bar:
Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=368396&signed_aid=_t4hsG2GFjiEe__8vT-LtQ==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=368397&signed_aid=q2ASdnVuMGpsPn9GAjrdmg==&inline=1

Location bar (no diff):
Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=368398&signed_aid=hPiZlkK4KcRMeaLejACitg==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=368399&signed_aid=q0PRCcZ5Fi0BRFv3oqcSQA==&inline=1

Bug: 788051
Change-Id: I54508125c77858bb8c40e80a0971fb6f9ddc9384
Reviewed-on: https://chromium-review.googlesource.com/c/1345672
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611493}
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/browser/ui/location_bar/location_bar.h
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/browser/ui/page_action/page_action_icon_container.h
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/browser/ui/views/frame/hosted_app_button_container.cc
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/browser/ui/views/frame/hosted_app_button_container.h
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/browser/ui/views/frame/toolbar_button_provider.h
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/browser/ui/views/page_action/page_action_icon_container_view.cc
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/browser/ui/views/page_action/page_action_icon_container_view.h
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/browser/ui/views/passwords/manage_passwords_icon_view_interactive_uitest.cc
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/browser/ui/views/passwords/manage_passwords_icon_views.cc
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/browser/ui/views/passwords/manage_passwords_icon_views.h
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/browser/ui/views/passwords/password_bubble_view_base.cc
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/browser/ui/views/toolbar/toolbar_view.cc
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/browser/ui/views/toolbar/toolbar_view.h
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/test/base/test_browser_window.cc
[modify] https://crrev.com/33845c065ed4ae2585eeb340175d9133ecf2e8a0/chrome/test/base/test_browser_window.h
Project Member

Comment 31 by bugdroid1@chromium.org, Nov 28 

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

commit 58906f9e202e2af8a7ba1de189df7defa3668d6a
Author: Alan Cutter <alancutter@chromium.org>
Date: Wed Nov 28 03:23:23 2018

Use Params struct to construct PageActionIconContainerView

The PageActionIconContainerView constructor takes 7 arguments.
This CL replaces them with a single Params struct for readability at the
call sites (in the absence of named function parameter passing in C++).

This CL has no changes in behaviour.

Bug: 788051
Change-Id: I423faf616c0f06236479c01579dd971c86251361
Reviewed-on: https://chromium-review.googlesource.com/c/1347640
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611545}
[modify] https://crrev.com/58906f9e202e2af8a7ba1de189df7defa3668d6a/chrome/browser/ui/views/frame/hosted_app_button_container.cc
[modify] https://crrev.com/58906f9e202e2af8a7ba1de189df7defa3668d6a/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/58906f9e202e2af8a7ba1de189df7defa3668d6a/chrome/browser/ui/views/page_action/page_action_icon_container_view.cc
[modify] https://crrev.com/58906f9e202e2af8a7ba1de189df7defa3668d6a/chrome/browser/ui/views/page_action/page_action_icon_container_view.h
►

Sign in to add a comment