New issue
Advanced search Search tips

Issue 728160 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug

Blocking:
issue 603386



Sign in to add a comment

[MacViews] Wire up FirstRunBubble

Project Member Reported by ellyjo...@chromium.org, May 31 2017

Issue description

This is invoked from LocationBarViewMac::ShowFirstRunBubbleInternal().
 
Owner: ellyjo...@chromium.org
Status: Assigned (was: Available)
Blocking: 603386
Labels: Phase3 Proj-MacViews
These should probably block MacViews/Harmony being on by default on Mac. Which I've been calling "Phase 3" --  Issue 603386 
The Cocoa bubble is so ugly.. 

Related:
  Issue 719117 
Status: Started (was: Assigned)
https://chromium-review.googlesource.com/c/535735
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 20 2017

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

commit 2410527ecf5e1baa7727ff9b08210b509912ae01
Author: Trent Apted <tapted@chromium.org>
Date: Thu Jul 20 23:53:49 2017

Consolidate PageInfo bubble anchoring logic.

Lots of things want to anchor off the page info icon. E.g.,
permission prompts, the device chooser UI, the first run bubble
and the page info bubble itself.

Unfortunately, the page info bubble is not always present (in
fullscreen for example), and we want to anchor toolkit-views bubbles
off a BrowserView as well as off a BrowserWindowCocoa. So anchoring
is not simple. Currently the logic that deals with this is scattered
around the codebase, and some of it is broken (e.g. crbug/745291).

This CL consolidates the anchoring for permission prompts and the
device chooser UI, for both of their Cocoa and Views versions. The
first run bubble will also use this in a follow-up.

The PageInfo bubble anchoring is a bit simpler since it can't be
shown in fullscreen. It's consolidated partially, but it has some
unnecessary plumbing as well, so more cleanups there are left for
another follow-up).

Bug:  728160 ,  745291 
Change-Id: Id8b3bb9284f8dcc83299aa92123d4ee6e58dddc4
Reviewed-on: https://chromium-review.googlesource.com/575791
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488494}
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/BUILD.gn
[add] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/bubble_anchor_util.h
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/cocoa/browser_dialogs_views_mac.cc
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/cocoa/browser_dialogs_views_mac.h
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/cocoa/bubble_anchor_helper.h
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/cocoa/bubble_anchor_helper.mm
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/cocoa/bubble_anchor_helper_views.h
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/cocoa/bubble_anchor_helper_views.mm
[add] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/cocoa/bubble_anchor_util_views_mac.mm
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/cocoa/page_info/page_info_bubble_controller.mm
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/cocoa/permission_bubble/chooser_bubble_ui_cocoa.mm
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/cocoa/permission_bubble/chooser_bubble_ui_views_mac.mm
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/cocoa/permission_bubble/permission_bubble_controller.mm
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/cocoa/permission_bubble/permission_bubble_controller_unittest.mm
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/cocoa/permission_bubble/permission_prompt_impl_views_mac.mm
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/cocoa/tab_dialogs_views_mac.mm
[add] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/views/bubble_anchor_util_views.cc
[add] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/views/bubble_anchor_util_views.h
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/views/permission_bubble/chooser_bubble_ui.cc
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/views/permission_bubble/chooser_bubble_ui.h
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/views/permission_bubble/chooser_bubble_ui_views.cc
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.h
[modify] https://crrev.com/2410527ecf5e1baa7727ff9b08210b509912ae01/chrome/browser/ui/views/permission_bubble/permission_prompt_impl_views.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 3 2017

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

commit fff01c727879f91438131a6d2ea01732e2524700
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Thu Aug 03 14:31:00 2017

MacViews: wire first run dialog

This change:
1) Makes FirstRunBubble::ShowBubble take an anchor point and a window to
   observe instead of both of these being implied by the anchor view;
2) Adds a class called FirstRunPresenter to share the logic used to wait
   to present the first run bubble between platforms;
3) Untangles LocationBar and ToolbarView from showing the first run
   bubble;
4) Adds chrome::ShowFirstRunBubbleViews() on Mac, to expose the Views
   first-run bubble

Bug:  728160 

Change-Id: I67de01f518a38120eb016ea2c6cd163dcf1c1356
Reviewed-on: https://chromium-review.googlesource.com/535735
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491730}
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/browser.cc
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/browser_dialogs.h
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/cocoa/browser_dialogs_views_mac.cc
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/cocoa/browser_dialogs_views_mac.h
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/cocoa/first_run_bubble_controller.h
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/cocoa/first_run_bubble_controller.mm
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/cocoa/first_run_bubble_controller_unittest.mm
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm
[add] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/first_run_bubble_presenter.cc
[add] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/first_run_bubble_presenter.h
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/location_bar/location_bar.h
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/views/browser_dialogs_views.cc
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/views/first_run_bubble.cc
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/views/first_run_bubble.h
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/views/first_run_bubble_unittest.cc
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/test/BUILD.gn
[modify] https://crrev.com/fff01c727879f91438131a6d2ea01732e2524700/chrome/test/base/test_browser_window.h

Status: Fixed (was: Started)

Sign in to add a comment