summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authortapted <tapted@chromium.org>2015-07-02 16:05:53 -0700
committerCommit bot <commit-bot@chromium.org>2015-07-02 23:06:36 +0000
commit982d085c7b6a26f3d8f5c94ed0cdf90873be0764 (patch)
tree03641aac8f646158f7141cd4f32d4ce3c47b13e8
parente4e094f274a21a97909fe3a11e79b4d48eabd7c4 (diff)
downloadchromium_src-982d085c7b6a26f3d8f5c94ed0cdf90873be0764.zip
chromium_src-982d085c7b6a26f3d8f5c94ed0cdf90873be0764.tar.gz
chromium_src-982d085c7b6a26f3d8f5c94ed0cdf90873be0764.tar.bz2
Revert of Mac: Fix missing focus rings on toolbar, profile, and tab buttons. (patchset #4 id:100001 of https://codereview.chromium.org/1160143010/)
Reason for revert: Tickles an AppKit Bug on OSX 10.8, causing a double free: [Cocoa Zombie] -[NSFocusStack fixInvalidatedFocusForFocusView] Original issue's description: > Mac: Fix missing focus rings on toolbar, profile, and tab buttons. > > In 10.7, AppKit introduced new API for drawing focus rings. > NSControl/NSCell subclasses can opt-in by overriding -focusRingMaskBounds and > -drawFocusRingMask. Implement these methods on the toolbar, profile, new tab and > tab close buttons so that they get focus rings when Full Keyboard Access is > enabled. > > But it turns out that this new focus ring does not work on buttons on 10.7, only on > editable text fields. They do work starting with 10.8. > > 10.6 before: manually drawn, tab close and avatar buttons missing focus rings. > 10.6 after: no change. > > 10.7 before: broken, no focus rings on toolbar, avatar, tab close, new tab buttons. > 10.7 after: manually drawn, same as 10.6. > > 10.8+ before: broken, no focus rings on toolbar, avatar, tab close, new tab buttons. > 10.8+ after: drawn by AppKit. > > Toolbar.xib changes: > Change the toolbar buttons (back, forward, reload, home, wrench) Focus Ring from > None to Default, to allow the new style focus ring to draw. > > BUG=459860 > > Committed: https://crrev.com/c72aa7b3cf651695deb3d781e99a9b122806949d > Cr-Commit-Position: refs/heads/master@{#334045} R=rsesek@chromium.org TBR=groby@chromium.org,andresantoso@chromium.org BUG=459860, 504808 Review URL: https://codereview.chromium.org/1220683005 Cr-Commit-Position: refs/heads/master@{#337262}
-rw-r--r--chrome/app/nibs/Toolbar.xib10
-rw-r--r--chrome/browser/ui/cocoa/autofill/down_arrow_popup_menu_cell.mm5
-rw-r--r--chrome/browser/ui/cocoa/hover_close_button.mm19
-rw-r--r--chrome/browser/ui/cocoa/image_button_cell.h3
-rw-r--r--chrome/browser/ui/cocoa/image_button_cell.mm24
-rw-r--r--chrome/browser/ui/cocoa/new_tab_button.mm13
-rw-r--r--chrome/browser/ui/cocoa/profiles/avatar_button_controller.mm12
-rw-r--r--chrome/browser/ui/cocoa/toolbar/toolbar_button_cocoa.mm12
8 files changed, 25 insertions, 73 deletions
diff --git a/chrome/app/nibs/Toolbar.xib b/chrome/app/nibs/Toolbar.xib
index b5e7b9a8..cde4f53 100644
--- a/chrome/app/nibs/Toolbar.xib
+++ b/chrome/app/nibs/Toolbar.xib
@@ -105,7 +105,7 @@
<bool key="NSEnabled">YES</bool>
<object class="NSButtonCell" key="NSCell" id="760046712">
<int key="NSCellFlags">67108864</int>
- <int key="NSCellFlags2">134217728</int>
+ <int key="NSCellFlags2">134250496</int>
<string key="NSContents"/>
<object class="NSFont" key="NSSupport" id="64724822">
<string key="NSName">LucidaGrande</string>
@@ -133,7 +133,7 @@
<bool key="NSEnabled">YES</bool>
<object class="NSButtonCell" key="NSCell" id="386107000">
<int key="NSCellFlags">67108864</int>
- <int key="NSCellFlags2">134217728</int>
+ <int key="NSCellFlags2">134250496</int>
<string key="NSContents"/>
<reference key="NSSupport" ref="64724822"/>
<reference key="NSControlView" ref="458854861"/>
@@ -157,7 +157,7 @@
<bool key="NSEnabled">YES</bool>
<object class="NSButtonCell" key="NSCell" id="3781855">
<int key="NSCellFlags">-2080374784</int>
- <int key="NSCellFlags2">134217728</int>
+ <int key="NSCellFlags2">134250496</int>
<string key="NSContents"/>
<reference key="NSSupport" ref="64724822"/>
<reference key="NSControlView" ref="781044416"/>
@@ -181,7 +181,7 @@
<bool key="NSEnabled">YES</bool>
<object class="NSButtonCell" key="NSCell" id="697431051">
<int key="NSCellFlags">-2080374784</int>
- <int key="NSCellFlags2">134217728</int>
+ <int key="NSCellFlags2">134250496</int>
<string key="NSContents"/>
<reference key="NSSupport" ref="64724822"/>
<reference key="NSControlView" ref="634265909"/>
@@ -204,7 +204,7 @@
<bool key="NSEnabled">YES</bool>
<object class="NSButtonCell" key="NSCell" id="204555298">
<int key="NSCellFlags">67108864</int>
- <int key="NSCellFlags2">134217728</int>
+ <int key="NSCellFlags2">134250496</int>
<string key="NSContents"/>
<reference key="NSSupport" ref="64724822"/>
<reference key="NSControlView" ref="602421009"/>
diff --git a/chrome/browser/ui/cocoa/autofill/down_arrow_popup_menu_cell.mm b/chrome/browser/ui/cocoa/autofill/down_arrow_popup_menu_cell.mm
index 43de6e9..0c8b46b 100644
--- a/chrome/browser/ui/cocoa/autofill/down_arrow_popup_menu_cell.mm
+++ b/chrome/browser/ui/cocoa/autofill/down_arrow_popup_menu_cell.mm
@@ -41,7 +41,10 @@ const int kSidePadding = 2.0;
if ([title length])
[self drawTitle:title withFrame:titleRect inView:controlView];
- [self drawFocusRingWithFrame:cellFrame inView:controlView];
+ // Only draw custom focus ring if the 10.7 focus ring APIs are not available.
+ // TODO(groby): Remove once we build against the 10.7 SDK.
+ if (![self respondsToSelector:@selector(drawFocusRingMaskWithFrame:inView:)])
+ [super drawFocusRingWithFrame:cellFrame inView:controlView];
}
@end
diff --git a/chrome/browser/ui/cocoa/hover_close_button.mm b/chrome/browser/ui/cocoa/hover_close_button.mm
index fbcb2ec..c2fed30 100644
--- a/chrome/browser/ui/cocoa/hover_close_button.mm
+++ b/chrome/browser/ui/cocoa/hover_close_button.mm
@@ -93,6 +93,14 @@ NSString* const kFadeOutValueKeyPath = @"fadeOutValue";
switch(self.hoverState) {
case kHoverStateMouseOver:
+ [image drawInRect:destRect
+ fromRect:imageRect
+ operation:NSCompositeSourceOver
+ fraction:1.0
+ respectFlipped:YES
+ hints:nil];
+ break;
+
case kHoverStateMouseDown:
[image drawInRect:destRect
fromRect:imageRect
@@ -129,17 +137,6 @@ NSString* const kFadeOutValueKeyPath = @"fadeOutValue";
}
}
-- (NSRect)focusRingMaskBounds {
- // This override won't be needed once we link with 10.8+ SDK.
- return [self bounds];
-}
-
-- (void)drawFocusRingMask {
- // Match the hover image's shape.
- NSRect circleRect = NSInsetRect([self bounds], 2, 2);
- [[NSBezierPath bezierPathWithOvalInRect:circleRect] fill];
-}
-
- (void)setFadeOutValue:(CGFloat)value {
[self setNeedsDisplay];
}
diff --git a/chrome/browser/ui/cocoa/image_button_cell.h b/chrome/browser/ui/cocoa/image_button_cell.h
index 7d93d43..e539284 100644
--- a/chrome/browser/ui/cocoa/image_button_cell.h
+++ b/chrome/browser/ui/cocoa/image_button_cell.h
@@ -75,9 +75,6 @@ enum ButtonState {
// Draws the cell's image within |cellFrame|.
- (void)drawImageWithFrame:(NSRect)cellFrame inView:(NSView*)controlView;
-// Draws |image| centered within |dstRect|.
-+ (void)drawImage:(NSImage*)image inRect:(NSRect)dstRect alpha:(CGFloat)alpha;
-
// If |controlView| is a first responder then draws a blue focus ring.
- (void)drawFocusRingWithFrame:(NSRect)cellFrame inView:(NSView*)controlView;
diff --git a/chrome/browser/ui/cocoa/image_button_cell.mm b/chrome/browser/ui/cocoa/image_button_cell.mm
index 1b43e70..ac15515 100644
--- a/chrome/browser/ui/cocoa/image_button_cell.mm
+++ b/chrome/browser/ui/cocoa/image_button_cell.mm
@@ -5,7 +5,6 @@
#import "chrome/browser/ui/cocoa/image_button_cell.h"
#include "base/logging.h"
-#include "base/mac/mac_util.h"
#import "chrome/browser/themes/theme_service.h"
#import "chrome/browser/ui/cocoa/rect_path_utils.h"
#import "chrome/browser/ui/cocoa/themed_window.h"
@@ -84,16 +83,12 @@ const CGFloat kImageNoFocusAlpha = 0.65;
}
}
- [ImageButtonCell drawImage:image inRect:cellFrame alpha:alpha];
-}
-
-+ (void)drawImage:(NSImage*)image inRect:(NSRect)dstRect alpha:(CGFloat)alpha {
NSRect imageRect;
imageRect.size = [image size];
- imageRect.origin.x =
- dstRect.origin.x + roundf((NSWidth(dstRect) - NSWidth(imageRect)) / 2.0);
- imageRect.origin.y = dstRect.origin.y +
- roundf((NSHeight(dstRect) - NSHeight(imageRect)) / 2.0);
+ imageRect.origin.x = cellFrame.origin.x +
+ roundf((NSWidth(cellFrame) - NSWidth(imageRect)) / 2.0);
+ imageRect.origin.y = cellFrame.origin.y +
+ roundf((NSHeight(cellFrame) - NSHeight(imageRect)) / 2.0);
[image drawInRect:imageRect
fromRect:NSZeroRect
@@ -105,7 +100,10 @@ const CGFloat kImageNoFocusAlpha = 0.65;
- (void)drawWithFrame:(NSRect)cellFrame inView:(NSView*)controlView {
[self drawImageWithFrame:cellFrame inView:controlView];
- [self drawFocusRingWithFrame:cellFrame inView:controlView];
+ // Only draw custom focus ring if the 10.7 focus ring APIs are not available.
+ // TODO(groby): Remove once we build against the 10.7 SDK.
+ if (![self respondsToSelector:@selector(drawFocusRingMaskWithFrame:inView:)])
+ [self drawFocusRingWithFrame:cellFrame inView:controlView];
}
- (void)setImageID:(NSInteger)imageID
@@ -139,12 +137,6 @@ const CGFloat kImageNoFocusAlpha = 0.65;
}
- (void)drawFocusRingWithFrame:(NSRect)cellFrame inView:(NSView*)controlView {
- // Draw custom focus ring only if AppKit won't draw one automatically.
- // The new focus ring APIs became available with 10.7, but did not get
- // applied to buttons (only editable text fields) until 10.8.
- if (base::mac::IsOSMountainLionOrLater())
- return;
-
if (![self showsFirstResponder])
return;
gfx::ScopedNSGraphicsContextSaveGState scoped_state;
diff --git a/chrome/browser/ui/cocoa/new_tab_button.mm b/chrome/browser/ui/cocoa/new_tab_button.mm
index 02fd53d..176c80c 100644
--- a/chrome/browser/ui/cocoa/new_tab_button.mm
+++ b/chrome/browser/ui/cocoa/new_tab_button.mm
@@ -56,19 +56,6 @@
return nil;
}
-- (NSRect)focusRingMaskBounds {
- // This override won't be needed once we link with 10.8+ SDK.
- return [self bounds];
-}
-
-- (void)drawFocusRingMask {
- // Match the button's shape.
- ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
- NSImage* image =
- bundle.GetNativeImageNamed(IDR_NEWTAB_BUTTON_MASK).ToNSImage();
- [ImageButtonCell drawImage:image inRect:[self bounds] alpha:1.0];
-}
-
// ThemedWindowDrawing implementation.
- (void)windowDidChangeTheme {
diff --git a/chrome/browser/ui/cocoa/profiles/avatar_button_controller.mm b/chrome/browser/ui/cocoa/profiles/avatar_button_controller.mm
index 50dae68..ba88a32 100644
--- a/chrome/browser/ui/cocoa/profiles/avatar_button_controller.mm
+++ b/chrome/browser/ui/cocoa/profiles/avatar_button_controller.mm
@@ -114,18 +114,6 @@ NSImage* GetImageFromResourceID(int resourceId) {
ui::DrawNinePartImage(frame, imageIds, NSCompositeSourceOver, 1.0, true);
}
-- (NSRect)focusRingMaskBoundsForFrame:(NSRect)cellFrame inView:(NSView*)view {
- // This override won't be needed once we link with 10.8+ SDK.
- return cellFrame;
-}
-
-- (void)drawFocusRingMaskWithFrame:(NSRect)frame inView:(NSView*)view {
- // Match the bezel's shape.
- [[NSBezierPath bezierPathWithRoundedRect:NSInsetRect(frame, 2, 2)
- xRadius:2
- yRadius:2] fill];
-}
-
- (void)setIsThemedWindow:(BOOL)isThemedWindow {
isThemedWindow_ = isThemedWindow;
}
diff --git a/chrome/browser/ui/cocoa/toolbar/toolbar_button_cocoa.mm b/chrome/browser/ui/cocoa/toolbar/toolbar_button_cocoa.mm
index 320e008..7e971cd 100644
--- a/chrome/browser/ui/cocoa/toolbar/toolbar_button_cocoa.mm
+++ b/chrome/browser/ui/cocoa/toolbar/toolbar_button_cocoa.mm
@@ -48,16 +48,4 @@
return handleMiddleClick_ && [theEvent buttonNumber] == 2;
}
-- (NSRect)focusRingMaskBounds {
- // This override won't be needed once we link with 10.8+ SDK.
- return [self bounds];
-}
-
-- (void)drawFocusRingMask {
- // Match the hover image's bezel.
- [[NSBezierPath bezierPathWithRoundedRect:NSInsetRect([self bounds], 2, 2)
- xRadius:2
- yRadius:2] fill];
-}
-
@end