Fix up fullscreen exit bubble messages to suggest Esc instead of F11 where appropriate.

Fix up fullscreen exit bubble messages to suggest Esc instead of F11 where appropriate.

Mac:
- Show "<link>Exit full screen</link> (Esc)". Since Mac only shows the bubble in tab fullscreen mode, it never needs to display the Cmd-Shift-F shortcut.

Win, Linux:
- Show "<link>Exit full screen (Esc)</link>". Will change it to match the mac message in a followup CL; this was easier for now.
- Show "Press Esc to exit fullscreen" when mouse is locked.
- Show "<link>Exit full screen (F11)</link>" when in browser fullscreen mode.

BUG=99869
TEST=manual


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110256

[jeremya, 2011-11-15 04:04:29]

Heya,
Nico- could you look at fullscreen_exit_bubble_controller.mm?

Vince- I reverted your link change to support the case of 'press esc to exit'
not being a link in mouse lock mode. Could you look through the Views and GTK
changes?

yzshen- I'm not sure I've handled all the states correctly. Can you see if I've
missed anything?

Thanks :)

[scheib, 2011-11-15 04:14:37]

> Vince- I reverted your link change to support the case of 'press esc to exit'
> not being a link in mouse lock mode. Could you look through the Views and GTK
> changes?

LGTM, though N.B. I'm not an expert on the UI code.

[Nico, 2011-11-15 05:22:01]

http://codereview.chromium.org/8528052/diff/2001/chrome/browser/ui/cocoa/full...
File chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller.mm (right):

http://codereview.chromium.org/8528052/diff/2001/chrome/browser/ui/cocoa/full...
chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller.mm:243: NSString
*exit_link_text = l10n_util::GetNSString(IDS_EXIT_FULLSCREEN_MODE);
Space after *, not before. Variables in ObjC called haveCamelCase.

http://codereview.chromium.org/8528052/diff/2001/chrome/browser/ui/cocoa/full...
chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller.mm:270:
textFrame.size.width = ceil(textFrame.size.width);
NSWidth() on the right

[jeremya, 2011-11-15 05:29:13]

http://codereview.chromium.org/8528052/diff/2001/chrome/browser/ui/cocoa/full...
File chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller.mm (right):

http://codereview.chromium.org/8528052/diff/2001/chrome/browser/ui/cocoa/full...
chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller.mm:243: NSString
*exit_link_text = l10n_util::GetNSString(IDS_EXIT_FULLSCREEN_MODE);
On 2011/11/15 05:22:02, Nico wrote:
> Space after *, not before. Variables in ObjC called haveCamelCase.

Done.

http://codereview.chromium.org/8528052/diff/2001/chrome/browser/ui/cocoa/full...
chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller.mm:270:
textFrame.size.width = ceil(textFrame.size.width);
On 2011/11/15 05:22:02, Nico wrote:
> NSWidth() on the right

Done.

[Nico, 2011-11-15 05:34:47]

lgtm

[yzshen1, 2011-11-15 21:11:29]

http://codereview.chromium.org/8528052/diff/3006/chrome/browser/ui/gtk/fullsc...
File chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc (right):

http://codereview.chromium.org/8528052/diff/3006/chrome/browser/ui/gtk/fullsc...
chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc:72:
gtk_chrome_link_button_set_label(GTK_CHROME_LINK_BUTTON(link_),
if |link_| is not visible, why you need to generate |exit_link_text| and set the
text of |link_|?

maybe line 68-73 can be moved into the if-block at line 74?

http://codereview.chromium.org/8528052/diff/3006/chrome/browser/ui/views/full...
File chrome/browser/ui/views/fullscreen_exit_bubble_views.cc (right):

http://codereview.chromium.org/8528052/diff/3006/chrome/browser/ui/views/full...
chrome/browser/ui/views/fullscreen_exit_bubble_views.cc:98:
link_.SetText(l10n_util::GetStringUTF16(IDS_EXIT_FULLSCREEN_MODE));
link_'s text will be set in UpdateContent, isn't it?
Maybe we don't need to set it here.

http://codereview.chromium.org/8528052/diff/3006/chrome/browser/ui/views/full...
chrome/browser/ui/views/fullscreen_exit_bubble_views.cc:195:
link_.SetText(l10n_util::GetStringUTF16(IDS_EXIT_FULLSCREEN_MODE));
You only need to set it when link_visible is true.
(Actually, if OS_CHROMEOS is defined, you only need to set it once.)

[jeremya, 2011-11-15 22:49:43]

http://codereview.chromium.org/8528052/diff/3006/chrome/browser/ui/gtk/fullsc...
File chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc (right):

http://codereview.chromium.org/8528052/diff/3006/chrome/browser/ui/gtk/fullsc...
chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc:72:
gtk_chrome_link_button_set_label(GTK_CHROME_LINK_BUTTON(link_),
On 2011/11/15 21:11:29, yzshen1 wrote:
> if |link_| is not visible, why you need to generate |exit_link_text| and set
the
> text of |link_|?
> 
> maybe line 68-73 can be moved into the if-block at line 74?

Done.

http://codereview.chromium.org/8528052/diff/3006/chrome/browser/ui/views/full...
File chrome/browser/ui/views/fullscreen_exit_bubble_views.cc (right):

http://codereview.chromium.org/8528052/diff/3006/chrome/browser/ui/views/full...
chrome/browser/ui/views/fullscreen_exit_bubble_views.cc:98:
link_.SetText(l10n_util::GetStringUTF16(IDS_EXIT_FULLSCREEN_MODE));
On 2011/11/15 21:11:29, yzshen1 wrote:
> link_'s text will be set in UpdateContent, isn't it?
> Maybe we don't need to set it here.

Done.

http://codereview.chromium.org/8528052/diff/3006/chrome/browser/ui/views/full...
chrome/browser/ui/views/fullscreen_exit_bubble_views.cc:195:
link_.SetText(l10n_util::GetStringUTF16(IDS_EXIT_FULLSCREEN_MODE));
On 2011/11/15 21:11:29, yzshen1 wrote:
> You only need to set it when link_visible is true.
> (Actually, if OS_CHROMEOS is defined, you only need to set it once.)

Done, I moved the OS_CHROMEOS part to the initializer.

[jeremya, 2011-11-15 22:51:25]

sky- need OWNERS rubber stamp :)

[yzshen1, 2011-11-15 22:54:57]

lgtm

[sky, 2011-11-16 02:59:19]

LGTM

[commit-bot: I haz the power, 2011-11-16 03:00:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeremya@chromium.org/8528052/13001

[commit-bot: I haz the power, 2011-11-16 04:39:06]

Change committed as 110256