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

Issue 200029: Make the bookmark toolbar folders act like a menu bar.... (Closed)

Created:
11 years, 3 months ago by Evan Stade
Modified:
9 years, 6 months ago
CC:
chromium-reviews_googlegroups.com, Ben Goodger (Google)
Visibility:
Public.

Description

Make the bookmark toolbar folders act like a menu bar. I didn't convert the page/app menu to use the new helper class in an effort to keep this patch small and reviewable. BUG=19675 BUG=15889 TEST=fiddled with it for while, and ran it under valgrind Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=25677

Patch Set 1 #

Patch Set 2 : lint #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+365 lines, -33 lines) Patch
M chrome/browser/gtk/bookmark_bar_gtk.h View 1 5 chunks +13 lines, -1 line 0 comments Download
M chrome/browser/gtk/bookmark_bar_gtk.cc View 11 chunks +74 lines, -23 lines 0 comments Download
M chrome/browser/gtk/bookmark_menu_controller_gtk.h View 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/gtk/bookmark_menu_controller_gtk.cc View 3 chunks +8 lines, -8 lines 0 comments Download
A chrome/browser/gtk/menu_bar_helper.h View 1 1 chunk +82 lines, -0 lines 0 comments Download
A chrome/browser/gtk/menu_bar_helper.cc View 1 chunk +181 lines, -0 lines 0 comments Download
M chrome/chrome.gyp View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/gtk_util.h View 1 chunk +2 lines, -0 lines 2 comments Download

Messages

Total messages: 4 (0 generated)
Evan Stade
11 years, 3 months ago (2009-09-05 00:09:36 UTC) #1
Evan Stade
ping
11 years, 3 months ago (2009-09-08 22:17:17 UTC) #2
Elliot Glaysher
LGTM :( Comment: This is really complicated and I dislike that we have to do ...
11 years, 3 months ago (2009-09-08 22:20:18 UTC) #3
Evan Stade
11 years, 3 months ago (2009-09-08 22:52:37 UTC) #4
yea, I know it sucks, that's one reason it's off in its own file (to quarantine
the poopiness)

http://codereview.chromium.org/200029/diff/3001/3006
File chrome/common/gtk_util.h (right):

http://codereview.chromium.org/200029/diff/3001/3006#newcode174
Line 174: void ConnectMenuActivatingWidgets(std::vector<GtkWidget*> widgets);
On 2009/09/08 22:20:18, Elliot Glaysher wrote:
> Stray declaration?

oh yeah woops this shouldn't be here

Powered by Google App Engine
This is Rietveld 408576698