Issue 199099: GTK: Set the back/forward and bookmark bar menus to always show their images....

Issue 199099: GTK: Set the back/forward and bookmark bar menus to always show their images.... (Closed)

Created:
11 years, 3 months ago by Evan Stade

Modified:
9 years, 6 months ago

Reviewers:
Evan Martin

CC:
chromium-reviews_googlegroups.com, Ben Goodger (Google)

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

Description

GTK: Set the back/forward and bookmark bar menus to always show their images. This only works for GTK 2.16+. BUG=21495 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=26043

Patch Set 1 #

Patch Set 2 : +comments #

Patch Set 3 : '' #

Total comments: 6

Patch Set 4 : commentary #

Created: 11 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+37 lines, -0 lines)			Patch
M	chrome/browser/gtk/back_forward_menu_model_gtk.h	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	chrome/browser/gtk/back_forward_menu_model_gtk.cc	View		1 chunk	+4 lines, -0 lines	0 comments	Download
M	chrome/browser/gtk/bookmark_menu_controller_gtk.cc	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	chrome/browser/gtk/menu_gtk.h	View	1 2 3	1 chunk	+3 lines, -0 lines	0 comments	Download
M	chrome/browser/gtk/menu_gtk.cc	View		1 chunk	+2 lines, -0 lines	0 comments	Download
M	chrome/common/gtk_util.h	View	1 2 3	1 chunk	+5 lines, -0 lines	0 comments	Download
M	chrome/common/gtk_util.cc	View	3	1 chunk	+21 lines, -0 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

Evan Martin

Can you update the review description to indicate it works on all GTKs now? http://codereview.chromium.org/199099/diff/4001/4006 ...

11 years, 3 months ago (2009-09-11 21:33:49 UTC) #4

Evan Stade

11 years, 3 months ago (2009-09-11 22:01:31 UTC) #6

fixed a bug (gtk_version_check() was returning the opposite of what I expected).

The description is correct; this only works for 2.16+. In 2.14 the
"always-show-image" property doesn't exist.

http://codereview.chromium.org/199099/diff/4001/4006
File chrome/browser/gtk/menu_gtk.h (right):

http://codereview.chromium.org/199099/diff/4001/4006#newcode46
Line 46: virtual bool AlwaysShowImages() const { return false; }
On 2009/09/11 21:33:49, Evan Martin wrote:
> Can you add docs to this?

Done.

http://codereview.chromium.org/199099/diff/4001/4008
File chrome/common/gtk_util.cc (right):

http://codereview.chromium.org/199099/diff/4001/4008#newcode482
Line 482: if (gtk_check_version(2, 16, 1)) {
On 2009/09/11 21:33:49, Evan Martin wrote:
> Can you add comments as to why this is necessary?

Done.

http://codereview.chromium.org/199099/diff/4001/4007
File chrome/common/gtk_util.h (right):

http://codereview.chromium.org/199099/diff/4001/4007#newcode175
Line 175: // show images.
On 2009/09/11 21:33:49, Evan Martin wrote:
> Maybe include "used for favicons" or something so people don't start using
this
> just because they think it looks better.

Done.

Expand Messages | Collapse Messages