Fix "open all" on a folder node to open only 'marks in that folder,...

Fix "open all" on a folder node to open only 'marks in that folder,
not ALL all.  Make Delete and Rename enabled for folder context menu
(but disabled for "Other Bookmarks").

nib changes (related to the context menus):
- Telegate is the bookmark bar controller.
- Switched type from NSMenu to BookmarkMenu.

TEST=New window.  Create folder "empty" with nothing in it.  Create
folder "two" with 2 bookmarks in it.  Create a final bookmark in the
top level of the bar.
Context menu (right click) over all folders.
Make sure Delete and Rename enabled for folders but not "Other Bookmarks".
Make sure "Open All" (and friends) disabled over "empty", enabled over "two".
"Open All" on "two" --> see 2 opened.
"Open All" on the bar itself --> see 3 opened.
Ditto for "Open all in new window" and "open all in incognito window".
Create a new window.  In new window, make sure folders have context menus.

BUG=http://crbug.com/27522, http://crbug.com/27529


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

[mrossetti, 2009-11-13 19:11:08]

LGTM with minor comments.

http://codereview.chromium.org/391046/diff/2001/2003
File chrome/browser/cocoa/bookmark_bar_controller.mm (right):

http://codereview.chromium.org/391046/diff/2001/2003#newcode50
Line 50: 
Got a bit blank line happy here?

http://codereview.chromium.org/391046/diff/2001/2003#newcode121
Line 121: // Then we set it's represented item to be the bookmark bar.
s/it's/its/

http://codereview.chromium.org/391046/diff/2001/2003#newcode319
Line 319: thingsToDo = [buttons_ count];
Shouldn't this be:
  thingsToDo = [buttons_ count] ? YES : NO;
?  Or is that only necessary for Boolean/bool conversions?

Alternatively:
  thingsToDo = [buttons_ count] > 0;

http://codereview.chromium.org/391046/diff/2001/2003#newcode331
Line 331: (!thingsToDo)) {
Since thingsToDo is a boolean, I might move it to the front of the expression to
avoid unnecessary, and more lengthy, selector compares.

http://codereview.chromium.org/391046/diff/2001/2010
File chrome/browser/cocoa/bookmark_menu.mm (right):

http://codereview.chromium.org/391046/diff/2001/2010#newcode14
Line 14: // is placed in a BookmarkMenu at the time a cell is asked for it's
s/it's/its/

http://codereview.chromium.org/391046/diff/2001/2009
File chrome/browser/cocoa/bookmark_menu_unittest.mm (right):

http://codereview.chromium.org/391046/diff/2001/2009#newcode15
Line 15: CocoaTestHelper cocoa_helper_;  // Inits Cocoa, creates window, etc...
To quote a buddy of mine: "See cocoa_test_helper.h for deprecation warning about
CocoaTestHeper"

[viettrungluu, 2009-11-13 21:19:45]

+1 on mrossetti's comments. Otherwise, this CL gives me the "there must be a
better way of doing this" feeling, but since I can't think of a better way
(which fits in with the current code), LGTM.

http://codereview.chromium.org/391046/diff/2001/2003
File chrome/browser/cocoa/bookmark_bar_controller.mm (right):

http://codereview.chromium.org/391046/diff/2001/2003#newcode319
Line 319: thingsToDo = [buttons_ count];
On 2009/11/13 19:11:09, mrossetti wrote:
> Shouldn't this be:
>   thingsToDo = [buttons_ count] ? YES : NO;
> ?  Or is that only necessary for Boolean/bool conversions?

I believe a BOOL is 8-bit, so the original code fails when there are 256 (or
multiples thereof) buttons.

> Alternatively:
>   thingsToDo = [buttons_ count] > 0;

This would work, though the expression on the right is a bool (not a BOOL).

[John Grabowski, 2009-11-13 21:29:18]

All changes applied.
New diffs up.

Now I lose a day since these files are all renamed due to non-M4 changes :-(

[pinkerton1, 2009-11-17 14:39:33]

What happened?

On Fri, Nov 13, 2009 at 1:29 PM,  <jrg@chromium.org> wrote:
> All changes applied.
> New diffs up.
>
> Now I lose a day since these files are all renamed due to non-M4 changes :-(
>
>
> http://codereview.chromium.org/391046
>



-- 
Mike Pinkerton
Mac Weenie
pinkerton@google.com