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

Issue 193033: Linux: expose "Use system title bar and borders" preference in options dialog. (Closed)

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

Description

Linux: expose "Use system title bar and borders" preference in options dialog. BUG=19483 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=25643

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -2 lines) Patch
M chrome/browser/gtk/options/content_page_gtk.h View 1 2 chunks +9 lines, -1 line 0 comments Download
M chrome/browser/gtk/options/content_page_gtk.cc View 1 5 chunks +26 lines, -1 line 1 comment Download

Messages

Total messages: 2 (0 generated)
Mike Mammarella
11 years, 3 months ago (2009-09-05 01:19:29 UTC) #1
Evan Stade
11 years, 3 months ago (2009-09-08 18:09:53 UTC) #2
lgtm after addressing the one comment

http://codereview.chromium.org/193033/diff/1003/1004
File chrome/browser/gtk/options/content_page_gtk.cc (right):

http://codereview.chromium.org/193033/diff/1003/1004#newcode64
Line 64: if (!pref_name || *pref_name == prefs::kPasswordManagerEnabled) {
I don't get the logic of these conditions, can't we just check pref_name once at
the top and be done with it?

it doesn't make sense to keep checking pref_name and besides it should be
(pref_name && ..) not (!pref_name || ...)

Powered by Google App Engine
This is Rietveld 408576698