Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(134)

Issue 160496: Implement back/forward toolbar menus on Mac.... (Closed)

Created:
11 years, 4 months ago by viettrungluu-gmail
Modified:
9 years, 7 months ago
Reviewers:
Avi (use Gerrit)
CC:
chromium-reviews_googlegroups.com, John Grabowski, Ben Goodger (Google)
Visibility:
Public.

Description

Implement back/forward toolbar menus on Mac (bug 13203). Note: The drop-down menu is actually a drag-down (activating on click-hold or on drag), working much like Safari's (and other Apple apps, such as Dictionary). This can be changed to a pop-down if that's what's desired. [The previously included fix to bug 17990 has been split off to CL 159864.] TODO: Show keyboard shortcut for "Show Full History". (No wait -- there *is* no keyboard shortcut for that on the Mac.) BUG=13203 TEST=Navigate around, check out and use the menus; do so in multiple tabs and windows.

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Total comments: 17

Patch Set 4 : '' #

Patch Set 5 : '' #

Patch Set 6 : '' #

Patch Set 7 : '' #

Patch Set 8 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+559 lines, -14 lines) Patch
M chrome/app/nibs/Toolbar.xib View 1 2 3 4 5 6 7 15 chunks +42 lines, -9 lines 0 comments Download
A chrome/browser/cocoa/back_forward_menu_controller.h View 1 2 3 1 chunk +42 lines, -0 lines 0 comments Download
A chrome/browser/cocoa/back_forward_menu_controller.mm View 1 2 3 1 chunk +100 lines, -0 lines 0 comments Download
M chrome/browser/cocoa/browser_window_controller.mm View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
A chrome/browser/cocoa/clickhold_button_cell.h View 1 chunk +45 lines, -0 lines 0 comments Download
A chrome/browser/cocoa/clickhold_button_cell.mm View 1 2 3 1 chunk +136 lines, -0 lines 0 comments Download
A chrome/browser/cocoa/delayedmenu_button.h View 1 2 3 4 5 6 1 chunk +30 lines, -0 lines 0 comments Download
A chrome/browser/cocoa/delayedmenu_button.mm View 1 2 3 4 5 6 1 chunk +129 lines, -0 lines 0 comments Download
M chrome/browser/cocoa/toolbar_button_cell.h View 1 2 3 4 5 6 7 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/cocoa/toolbar_controller.h View 1 2 3 4 5 6 7 4 chunks +12 lines, -3 lines 0 comments Download
M chrome/browser/cocoa/toolbar_controller.mm View 1 2 3 4 5 6 7 4 chunks +13 lines, -0 lines 0 comments Download
M chrome/browser/cocoa/toolbar_controller_unittest.mm View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M chrome/chrome.gyp View 1 2 3 4 5 6 7 2 chunks +6 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
Avi (use Gerrit)
http://codereview.chromium.org/160496/diff/1023/1029 File chrome/browser/cocoa/back_forward_menu_controller.h (right): http://codereview.chromium.org/160496/diff/1023/1029#newcode40 Line 40: @end // @interface something something? http://codereview.chromium.org/160496/diff/1023/1035 File chrome/browser/cocoa/back_forward_menu_controller.mm ...
11 years, 4 months ago (2009-08-04 15:04:35 UTC) #1
viettrungluu-gmail
Thanks, Avi. I split off the part requested into CL 159864. See my big comment ...
11 years, 4 months ago (2009-08-04 19:17:15 UTC) #2
Avi (use Gerrit)
http://codereview.chromium.org/160496/diff/1023/1034 File chrome/browser/cocoa/delayedmenu_button.h (right): http://codereview.chromium.org/160496/diff/1023/1034#newcode18 Line 18: // that the 0-th entry should be blank ...
11 years, 4 months ago (2009-08-04 19:29:34 UTC) #3
viettrungluu-gmail
http://codereview.chromium.org/160496/diff/1023/1034 File chrome/browser/cocoa/delayedmenu_button.h (right): http://codereview.chromium.org/160496/diff/1023/1034#newcode18 Line 18: // that the 0-th entry should be blank ...
11 years, 4 months ago (2009-08-04 19:54:18 UTC) #4
Avi (use Gerrit)
LG
11 years, 4 months ago (2009-08-04 19:55:45 UTC) #5
viettrungluu-gmail
I updated the patch (in particular chrome.gyp) versus the current ToT, and it still works. ...
11 years, 4 months ago (2009-08-05 06:24:19 UTC) #6
Avi (use Gerrit)
Committed r22478.
11 years, 4 months ago (2009-08-05 15:13:33 UTC) #7
Avi (use Gerrit)
On 2009/08/05 15:13:33, Avi wrote: > Committed r22478. Valgrind found some leaks. Investigating to see ...
11 years, 4 months ago (2009-08-05 16:54:22 UTC) #8
Avi (use Gerrit)
Valgrind filed as http://code.google.com/p/chromium/issues/detail?id=18532
11 years, 4 months ago (2009-08-05 18:02:30 UTC) #9
Avi (use Gerrit)
On 2009/08/05 18:02:30, Avi wrote: > Valgrind filed as http://code.google.com/p/chromium/issues/detail?id=18532 Reverting due to the valgrind ...
11 years, 4 months ago (2009-08-05 18:11:31 UTC) #10
viettrungluu-gmail
Patch Set 7 fixes the leak. Patch Set 8 (to be uploaded in a second) ...
11 years, 4 months ago (2009-08-05 21:47:06 UTC) #11
Avi (use Gerrit)
11 years, 4 months ago (2009-08-07 19:53:26 UTC) #12
Patched in set 8, but had to recreate the xib since it conflicted. Hope I got it
right :) but it worked for me.

Committed r22740; seems to pass all the tests.

Powered by Google App Engine
This is Rietveld 408576698