Removed the code that checks the tab number.

Removed the code that checks the tab number.
'Close Window' can be used even if a window has single tab.

NOTE: Cmd+w closes a window even if there is a single tab.

XIB Changes:
  * Make the default key equivalent of 'Close Tab' Cmd+w 
  * Make the default key equivalent of 'Close Window' Cmd+Shift+w
  * Remove the referencing outlets from 'Close Tab' and 'Close Window' 
No other changes are made. 

BUG=20569
TEST=press Cmd+Shift+w on a window having only one tab.


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

[shinyak (Google), 2011-06-03 06:23:18]

I have removed the code checking the tab number.
I think it is inconsistent that 'Close Tab' is not available when a single tab
is opened, as I commented in
http://code.google.com/p/chromium/issues/detail?id=20569

[Robert Sesek, 2011-06-03 15:46:51]

The window still closes if you hit Cmd+W with a single tab open, correct?

http://codereview.chromium.org/7065054/diff/1/chrome/app/nibs/MainMenu.xib
File chrome/app/nibs/MainMenu.xib (right):

http://codereview.chromium.org/7065054/diff/1/chrome/app/nibs/MainMenu.xib#ne...
chrome/app/nibs/MainMenu.xib:2: <archive
type="com.apple.InterfaceBuilder3.Cocoa.XIB" version="7.10">
Please list in the CL description all the XIB changes, like so:

"""
XIB Changes:
* Made outlet A go to B
* Changed action on C to be foo:
"""

http://codereview.chromium.org/7065054/diff/1/chrome/browser/app_controller_m...
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/7065054/diff/1/chrome/browser/app_controller_m...
chrome/browser/app_controller_mac.mm:363: // Capitalization of the key
equivalent affects whether the shift modifer is
Rewrap this comment to the previous line.

[shinyak (Google), 2011-06-03 16:41:45]

Let me do a short reply since I don't have an developing environment right
now.

> The window still closes if you hit Cmd+W with a single tab open, correct?

Yes, Cmd+W closes a window with a single tab open.

On Sat, Jun 4, 2011 at 12:46 AM, <rsesek@chromium.org> wrote:

> The window still closes if you hit Cmd+W with a single tab open, correct?
>
>
> http://codereview.chromium.org/7065054/diff/1/chrome/app/nibs/MainMenu.xib
> File chrome/app/nibs/MainMenu.xib (right):
>
>
>
http://codereview.chromium.org/7065054/diff/1/chrome/app/nibs/MainMenu.xib#ne...
> chrome/app/nibs/MainMenu.xib:2: <archive
> type="com.apple.InterfaceBuilder3.Cocoa.XIB" version="7.10">
> Please list in the CL description all the XIB changes, like so:
>
> """
> XIB Changes:
> * Made outlet A go to B
> * Changed action on C to be foo:
> """
>
>
>
http://codereview.chromium.org/7065054/diff/1/chrome/browser/app_controller_m...
> File chrome/browser/app_controller_mac.mm (right):
>
>
>
http://codereview.chromium.org/7065054/diff/1/chrome/browser/app_controller_m...
> chrome/browser/app_controller_mac.mm:363: // Capitalization of the key
> equivalent affects whether the shift modifer is
> Rewrap this comment to the previous line.
>
>
> http://codereview.chromium.org/7065054/
>



-- 
Shinya Kawanaka
shinyak@google.com

[shinyak (Google), 2011-06-04 06:35:46]

I've fixed the comment and description so that it includes XIB change.

[Robert Sesek, 2011-06-06 14:08:53]

LGTM

[Mark Mentovai, 2011-06-06 16:27:15]

http://codereview.chromium.org/7065054/diff/2003/chrome/browser/app_controlle...
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/7065054/diff/2003/chrome/browser/app_controlle...
chrome/browser/app_controller_mac.mm:365: [closeWindowMenuItem_
setKeyEquivalent:@"W"];
Your change comment indicates that this is Command-Shift-W maps to close-window
and Command-W maps to close-tab in the xib already. Are these two functions
still necessary?

[shinyak (Google), 2011-06-07 03:15:56]

http://codereview.chromium.org/7065054/diff/2003/chrome/browser/app_controlle...
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/7065054/diff/2003/chrome/browser/app_controlle...
chrome/browser/app_controller_mac.mm:365: [closeWindowMenuItem_
setKeyEquivalent:@"W"];
On 2011/06/06 16:27:15, Mark Mentovai wrote:
> Your change comment indicates that this is Command-Shift-W maps to
close-window
> and Command-W maps to close-tab in the xib already. Are these two functions
> still necessary?

This function is still used in fixCloseMenuItemKeyEquivalents, which is called
in delayedFixCloseMenuItemKeyEquivalents. It clears the key combination _to
prevent that from having any bad consequences_, and it is also required to
restore the original key combination. These functions is used to do so. So, they
are still necessary.

[Mark Mentovai, 2011-06-07 03:29:29]

Thanks for your response. I’ll clarify my concern.

http://codereview.chromium.org/7065054/diff/2003/chrome/browser/app_controlle...
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/7065054/diff/2003/chrome/browser/app_controlle...
chrome/browser/app_controller_mac.mm:365: [closeWindowMenuItem_
setKeyEquivalent:@"W"];
shinyak wrote:
> On 2011/06/06 16:27:15, Mark Mentovai wrote:
> > Your change comment indicates that this is Command-Shift-W maps to
> close-window
> > and Command-W maps to close-tab in the xib already. Are these two functions
> > still necessary?
> 
> This function is still used in fixCloseMenuItemKeyEquivalents, which is called
> in delayedFixCloseMenuItemKeyEquivalents. It clears the key combination _to
> prevent that from having any bad consequences_, and it is also required to
> restore the original key combination. These functions is used to do so. So,
they
> are still necessary.

Your explanation seems to gloss over the purpose of
-fixCloseMenuItemKeyEquivalents and -delayedFixCloseMenuItemKeyEquivalents, and
their use with -clearCloseMenuItemKeyEquivalents. Those machinations only exist
to support changing the key combinations on these menu items. If the menu items’
key combinations will no longer change, I question whether they’re necessary any
longer. If closeWindowMenuItem_ is never going to be triggered by anything other
than Command-W, and adjustCloseTabMenuItemKeyEquivalent is never going to be
triggered by anything other than Command-Shift-W, and there}s never any reason
to remove the key equivalents from either menu item, then why is all of this
necessary? The “bad consequences” that require -clearCloseMenuItemKeyEquivalents
only happen when the menu items’ key equivalents are changing.

[shinyak (Google), 2011-06-07 03:43:04]

http://codereview.chromium.org/7065054/diff/2003/chrome/browser/app_controlle...
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/7065054/diff/2003/chrome/browser/app_controlle...
chrome/browser/app_controller_mac.mm:365: [closeWindowMenuItem_
setKeyEquivalent:@"W"];
Ah, OK. I understand your comment. 
The menu items' key combination will not change any more. So I would like to try
to remove this code, too.

On 2011/06/07 03:29:29, Mark Mentovai wrote:
> shinyak wrote:
> > On 2011/06/06 16:27:15, Mark Mentovai wrote:
> > > Your change comment indicates that this is Command-Shift-W maps to
> > close-window
> > > and Command-W maps to close-tab in the xib already. Are these two
functions
> > > still necessary?
> > 
> > This function is still used in fixCloseMenuItemKeyEquivalents, which is
called
> > in delayedFixCloseMenuItemKeyEquivalents. It clears the key combination _to
> > prevent that from having any bad consequences_, and it is also required to
> > restore the original key combination. These functions is used to do so. So,
> they
> > are still necessary.
> 
> Your explanation seems to gloss over the purpose of
> -fixCloseMenuItemKeyEquivalents and -delayedFixCloseMenuItemKeyEquivalents,
and
> their use with -clearCloseMenuItemKeyEquivalents. Those machinations only
exist
> to support changing the key combinations on these menu items. If the menu
items’
> key combinations will no longer change, I question whether they’re necessary
any
> longer. If closeWindowMenuItem_ is never going to be triggered by anything
other
> than Command-W, and adjustCloseTabMenuItemKeyEquivalent is never going to be
> triggered by anything other than Command-Shift-W, and there}s never any reason
> to remove the key equivalents from either menu item, then why is all of this
> necessary? The “bad consequences” that require
-clearCloseMenuItemKeyEquivalents
> only happen when the menu items’ key equivalents are changing.

[Mark Mentovai, 2011-06-07 03:45:42]

Go for it!

As I see it, you ought to be able to remove a handful of methods and instance
variables from this class.

[Mark Mentovai, 2011-06-07 13:52:01]

LGTM

[commit-bot: I haz the power, 2011-06-07 16:26:04]

Change committed as 88133