Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(309)

Issue 502413004: Mac: Update sheet position when bookmark bar is shown or hidden. (Closed)

Created:
6 years, 3 months ago by Andre
Modified:
6 years, 3 months ago
Reviewers:
Avi (use Gerrit)
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Mac: Update sheet position when bookmark bar is shown or hidden. In r272421, I changed sheet repositioning on window resize to be based on NSWindowDidResizeNotification instead of NSViewFrameDidChangeNotification because it tracks window resizing much more smoothly. That caused this problem where if the browser window is vertically maximized (or is in fullscreen), the window size does not change when the bookmark bar is shown or hidden. The old code has a different bug however. If the browser window is not vertically maximized, showing or hiding the bookmark bar does not change the browser content view's frame. It moves up or down to follow the window resize but its frame relative to its superview remains the same. I settled for adding code to explicitly update the sheet's position when the bookmark bar is shown or hidden. BUG=407509 Committed: https://crrev.com/5648b809d1c50e3de754b96abe7cf2b60150ff4d Cr-Commit-Position: refs/heads/master@{#292421}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -0 lines) Patch
M chrome/browser/ui/cocoa/browser_window_controller.mm View 2 chunks +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.h View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.mm View 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Andre
andresantoso@chromium.org changed reviewers: + avi@chromium.org
6 years, 3 months ago (2014-08-27 01:07:54 UTC) #1
Andre
Avi, can you review?
6 years, 3 months ago (2014-08-28 05:20:58 UTC) #2
Avi (use Gerrit)
lgtm Sure. Simple and effective.
6 years, 3 months ago (2014-08-28 15:07:12 UTC) #3
Andre
The CQ bit was checked by andresantoso@chromium.org
6 years, 3 months ago (2014-08-28 16:17:31 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/andresantoso@chromium.org/502413004/1
6 years, 3 months ago (2014-08-28 16:18:19 UTC) #5
commit-bot: I haz the power
Committed patchset #1 (id:1) as dbcac30e9a43abba78b2e9ea96d1ae4694746e42
6 years, 3 months ago (2014-08-28 18:30:01 UTC) #6
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/5648b809d1c50e3de754b96abe7cf2b60150ff4d Cr-Commit-Position: refs/heads/master@{#292421}
6 years, 3 months ago (2014-09-10 03:00:50 UTC) #7
Andre
6 years, 2 months ago (2014-10-13 22:04:01 UTC) #8
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/653733002/ by andresantoso@chromium.org.

The reason for reverting is: Broke certificate info positioning when switching
tabs (http://crbug.com/419371).
I will need to come up with a better fix for this, but it looks complicated and
that bug is a M39 stable blocker so reverting this first.
.

Powered by Google App Engine
This is Rietveld 408576698