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

Issue 3310002: Gtk page info bubble. (Closed)

Created:
10 years, 3 months ago by mattm
Modified:
9 years, 7 months ago
CC:
chromium-reviews, ben+cc_chromium.org
Base URL:
git://codf21.jail/chromium.git
Visibility:
Public.

Description

Gtk page info bubble. BUG=52916 TEST=run chrome with --enable-new-page-info-bubble, click on page info Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=58249

Patch Set 1 #

Total comments: 8

Patch Set 2 : nits & const #

Patch Set 3 : remove comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+315 lines, -8 lines) Patch
M chrome/browser/gtk/location_bar_view_gtk.h View 1 1 chunk +3 lines, -0 lines 0 comments Download
A chrome/browser/gtk/page_info_bubble_gtk.cc View 1 2 1 chunk +311 lines, -0 lines 0 comments Download
M chrome/browser/gtk/page_info_window_gtk.cc View 1 1 chunk +0 lines, -8 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
mattm
Seems like it would be cleaner to change the ShowPageInfo API to also pass the ...
10 years, 3 months ago (2010-09-01 03:48:03 UTC) #1
Finnur
Screenshot looks good. I glanced at the code briefly and it look fine to me. ...
10 years, 3 months ago (2010-09-01 16:15:47 UTC) #2
Jay Civelli
LGTM http://codereview.chromium.org/3310002/diff/1/2 File chrome/browser/gtk/location_bar_view_gtk.h (right): http://codereview.chromium.org/3310002/diff/1/2#newcode62 chrome/browser/gtk/location_bar_view_gtk.h:62: GtkWidget* location_icon_widget() { return location_icon_image_; } Can it ...
10 years, 3 months ago (2010-09-01 16:29:25 UTC) #3
mattm
http://codereview.chromium.org/3310002/diff/1/2 File chrome/browser/gtk/location_bar_view_gtk.h (right): http://codereview.chromium.org/3310002/diff/1/2#newcode62 chrome/browser/gtk/location_bar_view_gtk.h:62: GtkWidget* location_icon_widget() { return location_icon_image_; } On 2010/09/01 16:29:25, ...
10 years, 3 months ago (2010-09-01 20:13:12 UTC) #4
Evan Stade
lgtm http://codereview.chromium.org/3310002/diff/1/3 File chrome/browser/gtk/page_info_bubble_gtk.cc (right): http://codereview.chromium.org/3310002/diff/1/3#newcode235 chrome/browser/gtk/page_info_bubble_gtk.cc:235: NOTREACHED(); // Do you need to add a ...
10 years, 3 months ago (2010-09-01 20:53:25 UTC) #5
Evan Stade
lgtm
10 years, 3 months ago (2010-09-01 20:53:25 UTC) #6
mattm
10 years, 3 months ago (2010-09-01 21:05:43 UTC) #7
http://codereview.chromium.org/3310002/diff/1/3
File chrome/browser/gtk/page_info_bubble_gtk.cc (right):

http://codereview.chromium.org/3310002/diff/1/3#newcode235
chrome/browser/gtk/page_info_bubble_gtk.cc:235: NOTREACHED();  // Do you need to
add a case here?
On 2010/09/01 20:53:25, Evan Stade wrote:
> what does this comment mean?

Hm, yeah.  That came from the views version.  I assume it means "if you hit this
notreached, maybe you need to add a case."  I'll just remove it.

Powered by Google App Engine
This is Rietveld 408576698