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

Issue 749214 link

Starred by 3 users

Issue metadata

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



Sign in to add a comment

[Regression] [macOS] Security Chip Bubble is misplaced in Fullscreen Mode when "Always Show Toolbar in Full Screen" is unchecked

Project Member Reported by meh...@chromium.org, Jul 26 2017

Issue description

Chrome Version: Canary Version 62.0.3167.0
OS: macOS 10.12.6

What steps will reproduce the problem?
(1) Uncheck "Always Show Toolbar in Full Screen" in Chrome Menubar
(2) Go into Fullscreen Mode with a window
(3) Visit google.com
(4) Click on the Security Chip, so that the Bubble appears

What is the expected result?
It should appear below the Security Chip.

What happens instead?
It appears in the left top corner.

Please use labels and text to provide additional information.

This is a regression. It works fine in Chrome Stable 60.0.3112.78.
 
Canary - not okay.png
88.5 KB View Download
Stable - okay.png
129 KB View Download

Comment 1 by shrike@chromium.org, Jul 27 2017

Owner: spqc...@chromium.org
spqchan@ - pleast take 
Cc: spqc...@chromium.org pnangunoori@chromium.org
Labels: -Needs-Bisect hasbisect-per-revision M-61
Owner: tapted@chromium.org
Status: Assigned (was: Untriaged)
Tested on latest Canary #62.0.3168.0 on Mac 10.12.5 and able to reproduce the issue. However issue is not reproduced on latest Chrome Stable #60.0.3112.78.

Please find the bisect info below:
Chrome Good Build - 61.0.3163.0 (488528)
Chrome Bad Build - 61.0.3162.0 (488073) 

You are probably looking for a change made after 488493 (known good), but no later than 488494 (first known bad).

CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.
https://chromium.googlesource.com/chromium/src/+log/ac20cda595c9955b20d015f6605ffeaa7cf00ec6..2410527ecf5e1baa7727ff9b08210b509912ae01

Review URL: https://chromium-review.googlesource.com/575791

From the commit above, assigning the issue to the owner concerned

@tapted -- Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to owner concerned.

Thanks.

Comment 3 by tapted@chromium.org, Jul 27 2017

Hm - I'll see if I can do something nicer here.

Note the "Stable ok" anchoring has kinda missed the mark too - the arrow is pointing at the wrong place. And for me the toolbar disappears sometimes, flakily, after clicking the site info button, which results in a "correct" anchoring at the top of the screen.

But it basically boils down to the logic in

  // If the browser is in browser-initiated full screen, a preference can cause
  // the toolbar to be hidden.
  if (browser->exclusive_access_manager()
          ->fullscreen_controller()
          ->IsFullscreenForBrowser()) {
    PrefService* prefs = browser->profile()->GetPrefs();
    bool show_toolbar = prefs->GetBoolean(prefs::kShowFullscreenToolbar);
    return show_toolbar;
  }

this also needs to check if the toolbar is revealed. (but that logic isn't new or changed by that CL, so it must have been getting determined some other way).

Comment 4 by tapted@chromium.org, Jul 31 2017

Status: Started (was: Assigned)
https://chromium-review.googlesource.com/593382
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 1 2017

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

commit f9cf932a6db81f79876f084b16426c14c0d2ffee
Author: Trent Apted <tapted@chromium.org>
Date: Tue Aug 01 06:09:54 2017

Cater for unrevealed toolbars when showing the PageInfoBubble on Mac

Currently, HasVisibleLocationBarForBrowser(..) just looks at the toolbar
visibility preference. Instead, query the toolbar controller to see whether
the toolbar has been revealed or not.

As an added bonus, permission bubbles are now able to re-anchor to the
toolbar/screen edge when revealing/hiding the toolbar in AppKit-
initiated fullscreen.

Bug:  749214 
Test: On Mac, Ensure View->"Always show toolbar in fullscreen" is unchecked.
Press Ctrl+Cmd+f to enter fullscreen.
Move the mouse to the top of the screen to reveal the toolbar and
click the 'Page Info' button in the location bar. The dialog should
open below the location bar rather than the top of the screen.

Change-Id: If7c0df7d3dc675f24cf467a2497ebe5c9cca7b8a
Reviewed-on: https://chromium-review.googlesource.com/593382
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490877}
[modify] https://crrev.com/f9cf932a6db81f79876f084b16426c14c0d2ffee/chrome/browser/ui/cocoa/bubble_anchor_helper.mm
[modify] https://crrev.com/f9cf932a6db81f79876f084b16426c14c0d2ffee/chrome/browser/ui/cocoa/permission_bubble/permission_bubble_cocoa_browser_test.mm

Labels: Merge-Request-61
Verified working in 62.0.3174.0. Requesting merge of r490877 to m61
Project Member

Comment 7 by sheriffbot@chromium.org, Aug 4 2017

Labels: -Merge-Request-61 Hotlist-Merge-Approved Merge-Approved-61
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop)

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

Comment 8 by bugdroid1@chromium.org, Aug 4 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/301c693e54181a1925b724aa59da1df78aac694c

commit 301c693e54181a1925b724aa59da1df78aac694c
Author: Trent Apted <tapted@chromium.org>
Date: Fri Aug 04 04:22:46 2017

[merge-m60] Cater for unrevealed toolbars when showing the PageInfoBubble on Mac

Currently, HasVisibleLocationBarForBrowser(..) just looks at the toolbar
visibility preference. Instead, query the toolbar controller to see whether
the toolbar has been revealed or not.

As an added bonus, permission bubbles are now able to re-anchor to the
toolbar/screen edge when revealing/hiding the toolbar in AppKit-
initiated fullscreen.

Bug:  749214 
Test: On Mac, Ensure View->"Always show toolbar in fullscreen" is unchecked.
Press Ctrl+Cmd+f to enter fullscreen.
Move the mouse to the top of the screen to reveal the toolbar and
click the 'Page Info' button in the location bar. The dialog should
open below the location bar rather than the top of the screen.

TBR=tapted@chromium.org

(cherry picked from commit f9cf932a6db81f79876f084b16426c14c0d2ffee)

Change-Id: If7c0df7d3dc675f24cf467a2497ebe5c9cca7b8a
Reviewed-on: https://chromium-review.googlesource.com/593382
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#490877}
Reviewed-on: https://chromium-review.googlesource.com/601608
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#306}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/301c693e54181a1925b724aa59da1df78aac694c/chrome/browser/ui/cocoa/bubble_anchor_helper.mm
[modify] https://crrev.com/301c693e54181a1925b724aa59da1df78aac694c/chrome/browser/ui/cocoa/permission_bubble/permission_bubble_cocoa_browser_test.mm

Status: Fixed (was: Started)
Cc: ellyjo...@chromium.org nyerramilli@chromium.org tapted@chromium.org ashej...@chromium.org
 Issue 608245  has been merged into this issue.

Sign in to add a comment