Commit aaabc160 authored by Avi Drissman's avatar Avi Drissman Committed by Commit Bot

Fix quit confirmation panel for non-US keyboards

Apple’s documentation on menus is incorrect (see the linked bug)
so the test we were doing was wrong. And anyway, why double-check
the shortcut key? We're in -applicationShouldTerminate: so the
user *did* give a quit command. Just assume that the system did
the right thing and match on the fact that we're processing a
key down.

Also do some cleanup of comments and names that are just wrong.

BUG=142944

Change-Id: I797a97d3675e67e2426cc296e7c8642e08b91504
Reviewed-on: https://chromium-review.googlesource.com/c/1345030Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610853}
parent e3da3b00
...@@ -538,7 +538,7 @@ static base::mac::ScopedObjCClassSwizzler* g_swizzle_imk_input_session; ...@@ -538,7 +538,7 @@ static base::mac::ScopedObjCClassSwizzler* g_swizzle_imk_input_session;
// http://dev.chromium.org/developers/design-documents/confirm-to-quit-experiment // http://dev.chromium.org/developers/design-documents/confirm-to-quit-experiment
// This logic is only for keyboard-initiated quits. // This logic is only for keyboard-initiated quits.
if (![ConfirmQuitPanelController eventTriggersFeature:[app currentEvent]]) if ([[app currentEvent] type] != NSKeyDown)
return NSTerminateNow; return NSTerminateNow;
return [[ConfirmQuitPanelController sharedController] return [[ConfirmQuitPanelController sharedController]
......
...@@ -21,9 +21,6 @@ ...@@ -21,9 +21,6 @@
// does not currently exist. // does not currently exist.
+ (ConfirmQuitPanelController*)sharedController; + (ConfirmQuitPanelController*)sharedController;
// Checks whether the |event| should trigger the feature.
+ (BOOL)eventTriggersFeature:(NSEvent*)event;
// Runs a modal loop that brings up the panel and handles the logic for if and // Runs a modal loop that brings up the panel and handles the logic for if and
// when to terminate. Returns NSApplicationTerminateReply for use in // when to terminate. Returns NSApplicationTerminateReply for use in
// -[NSApplicationDelegate applicationShouldTerminate:]. // -[NSApplicationDelegate applicationShouldTerminate:].
...@@ -36,13 +33,13 @@ ...@@ -36,13 +33,13 @@
// instructions on how to quit. // instructions on how to quit.
- (void)dismissPanel; - (void)dismissPanel;
// Returns a string representation fit for display of |+quitAccelerator|. // Returns a string representation fit for display.
+ (NSString*)keyCommandString; + (NSString*)keyCommandString;
@end @end
@interface ConfirmQuitPanelController (UnitTesting) @interface ConfirmQuitPanelController (UnitTesting)
+ (NSString*)keyCombinationForAccelerator:(NSMenuItem*)item; + (NSString*)keyCombinationForMenuItem:(NSMenuItem*)item;
@end @end
#endif // CHROME_BROWSER_UI_COCOA_CONFIRM_QUIT_PANEL_CONTROLLER_H_ #endif // CHROME_BROWSER_UI_COCOA_CONFIRM_QUIT_PANEL_CONTROLLER_H_
...@@ -151,8 +151,9 @@ const NSTimeInterval kTimeDeltaFuzzFactor = 1.0; ...@@ -151,8 +151,9 @@ const NSTimeInterval kTimeDeltaFuzzFactor = 1.0;
- (NSEvent*)pumpEventQueueForKeyUp:(NSApplication*)app untilDate:(NSDate*)date; - (NSEvent*)pumpEventQueueForKeyUp:(NSApplication*)app untilDate:(NSDate*)date;
- (void)hideAllWindowsForApplication:(NSApplication*)app - (void)hideAllWindowsForApplication:(NSApplication*)app
withDuration:(NSTimeInterval)duration; withDuration:(NSTimeInterval)duration;
// Returns the Accelerator for the Quit menu item. // Returns the menu item for the Quit menu item, or a thrown-together default
+ (NSMenuItem*)quitAccelerator; // one if no Quit menu item exists.
+ (NSMenuItem*)quitMenuItem;
@end @end
ConfirmQuitPanelController* g_confirmQuitPanelController = nil; ConfirmQuitPanelController* g_confirmQuitPanelController = nil;
...@@ -197,16 +198,6 @@ ConfirmQuitPanelController* g_confirmQuitPanelController = nil; ...@@ -197,16 +198,6 @@ ConfirmQuitPanelController* g_confirmQuitPanelController = nil;
return self; return self;
} }
+ (BOOL)eventTriggersFeature:(NSEvent*)event {
if ([event type] != NSKeyDown)
return NO;
NSMenuItem* item = [self quitAccelerator];
return item.keyEquivalentModifierMask ==
([event modifierFlags] & NSDeviceIndependentModifierFlagsMask) &&
[item.keyEquivalent
isEqualToString:[event charactersIgnoringModifiers]];
}
- (NSApplicationTerminateReply)runModalLoopForApplication:(NSApplication*)app { - (NSApplicationTerminateReply)runModalLoopForApplication:(NSApplication*)app {
base::scoped_nsobject<ConfirmQuitPanelController> keepAlive([self retain]); base::scoped_nsobject<ConfirmQuitPanelController> keepAlive([self retain]);
...@@ -347,7 +338,7 @@ ConfirmQuitPanelController* g_confirmQuitPanelController = nil; ...@@ -347,7 +338,7 @@ ConfirmQuitPanelController* g_confirmQuitPanelController = nil;
// key combination for quit. It then gets the modifiers and builds a string // key combination for quit. It then gets the modifiers and builds a string
// to display them. // to display them.
+ (NSString*)keyCommandString { + (NSString*)keyCommandString {
return [[self class] keyCombinationForAccelerator:[self quitAccelerator]]; return [[self class] keyCombinationForMenuItem:[self quitMenuItem]];
} }
// Runs a nested loop that pumps the event queue until the next KeyUp event. // Runs a nested loop that pumps the event queue until the next KeyUp event.
...@@ -368,10 +359,8 @@ ConfirmQuitPanelController* g_confirmQuitPanelController = nil; ...@@ -368,10 +359,8 @@ ConfirmQuitPanelController* g_confirmQuitPanelController = nil;
[animation startAnimation]; [animation startAnimation];
} }
// This looks at the Main Menu and determines what the user has set as the // This returns the NSMenuItem that quits the application.
// key combination for quit. It then gets the modifiers and builds an object + (NSMenuItem*)quitMenuItem {
// to hold the data.
+ (NSMenuItem*)quitAccelerator {
NSMenu* mainMenu = [NSApp mainMenu]; NSMenu* mainMenu = [NSApp mainMenu];
// Get the application menu (i.e. Chromium). // Get the application menu (i.e. Chromium).
NSMenu* appMenu = [[mainMenu itemAtIndex:0] submenu]; NSMenu* appMenu = [[mainMenu itemAtIndex:0] submenu];
...@@ -390,7 +379,7 @@ ConfirmQuitPanelController* g_confirmQuitPanelController = nil; ...@@ -390,7 +379,7 @@ ConfirmQuitPanelController* g_confirmQuitPanelController = nil;
return item; return item;
} }
+ (NSString*)keyCombinationForAccelerator:(NSMenuItem*)item { + (NSString*)keyCombinationForMenuItem:(NSMenuItem*)item {
NSMutableString* string = [NSMutableString string]; NSMutableString* string = [NSMutableString string];
NSUInteger modifiers = item.keyEquivalentModifierMask; NSUInteger modifiers = item.keyEquivalentModifierMask;
......
...@@ -40,7 +40,7 @@ TEST_F(ConfirmQuitPanelControllerTest, ShowAndDismiss) { ...@@ -40,7 +40,7 @@ TEST_F(ConfirmQuitPanelControllerTest, ShowAndDismiss) {
EXPECT_EQ(controller, [ConfirmQuitPanelController sharedController]); EXPECT_EQ(controller, [ConfirmQuitPanelController sharedController]);
} }
TEST_F(ConfirmQuitPanelControllerTest, KeyCombinationForAccelerator) { TEST_F(ConfirmQuitPanelControllerTest, KeyCombinationForMenuItem) {
Class controller = [ConfirmQuitPanelController class]; Class controller = [ConfirmQuitPanelController class];
NSMenuItem* item = [[[NSMenuItem alloc] initWithTitle:@"" NSMenuItem* item = [[[NSMenuItem alloc] initWithTitle:@""
...@@ -49,39 +49,39 @@ TEST_F(ConfirmQuitPanelControllerTest, KeyCombinationForAccelerator) { ...@@ -49,39 +49,39 @@ TEST_F(ConfirmQuitPanelControllerTest, KeyCombinationForAccelerator) {
item.keyEquivalent = @"q"; item.keyEquivalent = @"q";
item.keyEquivalentModifierMask = NSCommandKeyMask; item.keyEquivalentModifierMask = NSCommandKeyMask;
EXPECT_NSEQ(TestString(@"{Cmd}Q"), EXPECT_NSEQ(TestString(@"{Cmd}Q"),
[controller keyCombinationForAccelerator:item]); [controller keyCombinationForMenuItem:item]);
item.keyEquivalent = @"c"; item.keyEquivalent = @"c";
item.keyEquivalentModifierMask = NSCommandKeyMask | NSShiftKeyMask; item.keyEquivalentModifierMask = NSCommandKeyMask | NSShiftKeyMask;
EXPECT_NSEQ(TestString(@"{Cmd}{Shift}C"), EXPECT_NSEQ(TestString(@"{Cmd}{Shift}C"),
[controller keyCombinationForAccelerator:item]); [controller keyCombinationForMenuItem:item]);
item.keyEquivalent = @"h"; item.keyEquivalent = @"h";
item.keyEquivalentModifierMask = item.keyEquivalentModifierMask =
NSCommandKeyMask | NSShiftKeyMask | NSAlternateKeyMask; NSCommandKeyMask | NSShiftKeyMask | NSAlternateKeyMask;
EXPECT_NSEQ(TestString(@"{Cmd}{Opt}{Shift}H"), EXPECT_NSEQ(TestString(@"{Cmd}{Opt}{Shift}H"),
[controller keyCombinationForAccelerator:item]); [controller keyCombinationForMenuItem:item]);
item.keyEquivalent = @"r"; item.keyEquivalent = @"r";
item.keyEquivalentModifierMask = item.keyEquivalentModifierMask =
NSCommandKeyMask | NSShiftKeyMask | NSAlternateKeyMask | NSControlKeyMask; NSCommandKeyMask | NSShiftKeyMask | NSAlternateKeyMask | NSControlKeyMask;
EXPECT_NSEQ(TestString(@"{Cmd}{Ctrl}{Opt}{Shift}R"), EXPECT_NSEQ(TestString(@"{Cmd}{Ctrl}{Opt}{Shift}R"),
[controller keyCombinationForAccelerator:item]); [controller keyCombinationForMenuItem:item]);
item.keyEquivalent = @"o"; item.keyEquivalent = @"o";
item.keyEquivalentModifierMask = NSControlKeyMask; item.keyEquivalentModifierMask = NSControlKeyMask;
EXPECT_NSEQ(TestString(@"{Ctrl}O"), EXPECT_NSEQ(TestString(@"{Ctrl}O"),
[controller keyCombinationForAccelerator:item]); [controller keyCombinationForMenuItem:item]);
item.keyEquivalent = @"m"; item.keyEquivalent = @"m";
item.keyEquivalentModifierMask = NSShiftKeyMask | NSControlKeyMask; item.keyEquivalentModifierMask = NSShiftKeyMask | NSControlKeyMask;
EXPECT_NSEQ(TestString(@"{Ctrl}{Shift}M"), EXPECT_NSEQ(TestString(@"{Ctrl}{Shift}M"),
[controller keyCombinationForAccelerator:item]); [controller keyCombinationForMenuItem:item]);
item.keyEquivalent = @"e"; item.keyEquivalent = @"e";
item.keyEquivalentModifierMask = NSCommandKeyMask | NSAlternateKeyMask; item.keyEquivalentModifierMask = NSCommandKeyMask | NSAlternateKeyMask;
EXPECT_NSEQ(TestString(@"{Cmd}{Opt}E"), EXPECT_NSEQ(TestString(@"{Cmd}{Opt}E"),
[controller keyCombinationForAccelerator:item]); [controller keyCombinationForMenuItem:item]);
} }
} // namespace } // namespace
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