Commit b5a19d4f authored by erikchen's avatar erikchen Committed by Commit bot

mac: Fix bug where finder window extends past bottom of screen.

AppKit is responsible for determining the height of the NSSavePanel and
NSOpenPanel. It will never pick a height that causes the panel to be taller
than the screen, but it may pick a height that requires the panel to be
positioned near the top of its presenting window. Chrome's logic for
positioning panels did not take this into consideration, so it was possible to
position a panel that extended past the bottom of the screen.

This CL fixes this oversight.

BUG=423635

Review URL: https://codereview.chromium.org/665933005

Cr-Commit-Position: refs/heads/master@{#301533}
parent 60c10816
...@@ -40,11 +40,21 @@ ...@@ -40,11 +40,21 @@
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "content/public/test/test_utils.h" #include "content/public/test/test_utils.h"
#import "testing/gtest_mac.h" #import "testing/gtest_mac.h"
#import "third_party/ocmock/OCMock/OCMock.h"
#import "ui/base/cocoa/nsview_additions.h" #import "ui/base/cocoa/nsview_additions.h"
#include "ui/gfx/animation/slide_animation.h" #include "ui/gfx/animation/slide_animation.h"
namespace { namespace {
// Creates a mock of an NSWindow that has the given |frame|.
id MockWindowWithFrame(NSRect frame) {
id window = [OCMockObject mockForClass:[NSWindow class]];
NSValue* window_frame =
[NSValue valueWithBytes:&frame objCType:@encode(NSRect)];
[[[window stub] andReturnValue:window_frame] frame];
return window;
}
void CreateProfileCallback(const base::Closure& quit_closure, void CreateProfileCallback(const base::Closure& quit_closure,
Profile* profile, Profile* profile,
Profile::CreateStatus status) { Profile::CreateStatus status) {
...@@ -482,6 +492,7 @@ IN_PROC_BROWSER_TEST_F(BrowserWindowControllerTest, SheetPosition) { ...@@ -482,6 +492,7 @@ IN_PROC_BROWSER_TEST_F(BrowserWindowControllerTest, SheetPosition) {
EXPECT_FALSE([controller() isBookmarkBarVisible]); EXPECT_FALSE([controller() isBookmarkBarVisible]);
NSRect defaultAlertFrame = NSMakeRect(0, 0, 300, 200); NSRect defaultAlertFrame = NSMakeRect(0, 0, 300, 200);
id sheet = MockWindowWithFrame(defaultAlertFrame);
NSWindow* window = browser()->window()->GetNativeWindow(); NSWindow* window = browser()->window()->GetNativeWindow();
NSRect alertFrame = [controller() window:window NSRect alertFrame = [controller() window:window
willPositionSheet:nil willPositionSheet:nil
...@@ -493,11 +504,24 @@ IN_PROC_BROWSER_TEST_F(BrowserWindowControllerTest, SheetPosition) { ...@@ -493,11 +504,24 @@ IN_PROC_BROWSER_TEST_F(BrowserWindowControllerTest, SheetPosition) {
chrome::ToggleBookmarkBarWhenVisible(browser()->profile()); chrome::ToggleBookmarkBarWhenVisible(browser()->profile());
EXPECT_TRUE([controller() isBookmarkBarVisible]); EXPECT_TRUE([controller() isBookmarkBarVisible]);
alertFrame = [controller() window:window alertFrame = [controller() window:window
willPositionSheet:nil willPositionSheet:sheet
usingRect:defaultAlertFrame]; usingRect:defaultAlertFrame];
NSRect bookmarkBarFrame = [[[controller() bookmarkBarController] view] frame]; NSRect bookmarkBarFrame = [[[controller() bookmarkBarController] view] frame];
EXPECT_EQ(NSMinY(alertFrame), NSMinY(bookmarkBarFrame)); EXPECT_EQ(NSMinY(alertFrame), NSMinY(bookmarkBarFrame));
// If the sheet is too large, it should be positioned at the top of the
// window.
defaultAlertFrame = NSMakeRect(0, 0, 300, 2000);
sheet = MockWindowWithFrame(defaultAlertFrame);
alertFrame = [controller() window:window
willPositionSheet:sheet
usingRect:defaultAlertFrame];
EXPECT_EQ(NSMinY(alertFrame), NSHeight([window frame]));
// Reset the sheet's size.
defaultAlertFrame = NSMakeRect(0, 0, 300, 200);
sheet = MockWindowWithFrame(defaultAlertFrame);
// Make sure the profile does not have the bookmark visible so that // Make sure the profile does not have the bookmark visible so that
// we'll create the shortcut window without the bookmark bar. // we'll create the shortcut window without the bookmark bar.
chrome::ToggleBookmarkBarWhenVisible(browser()->profile()); chrome::ToggleBookmarkBarWhenVisible(browser()->profile());
...@@ -518,7 +542,7 @@ IN_PROC_BROWSER_TEST_F(BrowserWindowControllerTest, SheetPosition) { ...@@ -518,7 +542,7 @@ IN_PROC_BROWSER_TEST_F(BrowserWindowControllerTest, SheetPosition) {
// Open sheet in an application window. // Open sheet in an application window.
[popupController showWindow:nil]; [popupController showWindow:nil];
alertFrame = [popupController window:popupWindow alertFrame = [popupController window:popupWindow
willPositionSheet:nil willPositionSheet:sheet
usingRect:defaultAlertFrame]; usingRect:defaultAlertFrame];
EXPECT_EQ(NSMinY(alertFrame), EXPECT_EQ(NSMinY(alertFrame),
NSHeight([[popupWindow contentView] frame]) - NSHeight([[popupWindow contentView] frame]) -
......
...@@ -180,26 +180,44 @@ willPositionSheet:(NSWindow*)sheet ...@@ -180,26 +180,44 @@ willPositionSheet:(NSWindow*)sheet
// the sheet below the bookmark bar. // the sheet below the bookmark bar.
// - If the bookmark bar is currently animating, position the sheet according // - If the bookmark bar is currently animating, position the sheet according
// to where the bar will be when the animation ends. // to where the bar will be when the animation ends.
CGFloat defaultSheetY = defaultSheetRect.origin.y;
switch ([bookmarkBarController_ currentState]) { switch ([bookmarkBarController_ currentState]) {
case BookmarkBar::SHOW: { case BookmarkBar::SHOW: {
NSRect bookmarkBarFrame = [[bookmarkBarController_ view] frame]; NSRect bookmarkBarFrame = [[bookmarkBarController_ view] frame];
defaultSheetRect.origin.y = bookmarkBarFrame.origin.y; defaultSheetY = bookmarkBarFrame.origin.y;
break; break;
} }
case BookmarkBar::HIDDEN: case BookmarkBar::HIDDEN:
case BookmarkBar::DETACHED: { case BookmarkBar::DETACHED: {
if ([self hasToolbar]) { if ([self hasToolbar]) {
NSRect toolbarFrame = [[toolbarController_ view] frame]; NSRect toolbarFrame = [[toolbarController_ view] frame];
defaultSheetRect.origin.y = toolbarFrame.origin.y; defaultSheetY = toolbarFrame.origin.y;
} else { } else {
// The toolbar is not shown in application mode. The sheet should be // The toolbar is not shown in application mode. The sheet should be
// located at the top of the window, under the title of the window. // located at the top of the window, under the title of the window.
defaultSheetRect.origin.y = NSHeight([[window contentView] frame]) - defaultSheetY = NSHeight([[window contentView] frame]) -
defaultSheetRect.size.height; defaultSheetRect.size.height;
} }
break; break;
} }
} }
// AppKit may shift the window up to fit the sheet on screen, but it will
// never adjust the height of the sheet, or the origin of the sheet relative
// to the window. Adjust the origin to prevent sheets from extending past the
// bottom of the screen.
// Don't allow the sheet to extend past the bottom of the window. This logic
// intentionally ignores the size of the screens, since the window might span
// multiple screens, and AppKit may reposition the window.
CGFloat sheetHeight = NSHeight([sheet frame]);
defaultSheetY = std::max(defaultSheetY, sheetHeight);
// It doesn't make sense to provide a Y higher than the height of the window.
CGFloat windowHeight = NSHeight([window frame]);
defaultSheetY = std::min(defaultSheetY, windowHeight);
defaultSheetRect.origin.y = defaultSheetY;
return defaultSheetRect; return defaultSheetRect;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment