Gtk: Change content settings window to use a list instead of tabs.

Gtk: Change content settings window to use a list instead of tabs. 

BUG=46965
TEST=Open content settings window. Should look more like it does on windows.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=51869

[Nico, 2010-07-06 21:53:59]

Screenshots (3 of them): http://imgur.com/WdGZgl&gz9yt&nD3bK

[Nico, 2010-07-06 21:54:53]

+correct erg

[Evan Martin, 2010-07-07 15:44:07]

http://codereview.chromium.org/2757005/diff/10001/11002
File chrome/browser/gtk/options/content_settings_window_gtk.cc (right):

http://codereview.chromium.org/2757005/diff/10001/11002#newcode28
chrome/browser/gtk/options/content_settings_window_gtk.cc:28: #include
"chrome/browser/gtk/gtk_tree.h"
Why not include with the rest?

http://codereview.chromium.org/2757005/diff/10001/11002#newcode108
chrome/browser/gtk/options/content_settings_window_gtk.cc:108: GtkWidget* hbox =
gtk_hbox_new(FALSE, gtk_util::kControlSpacing);
Can you use the wider spacing for this?

http://codereview.chromium.org/2757005/diff/10001/11002#newcode163
chrome/browser/gtk/options/content_settings_window_gtk.cc:163:
l10n_util::GetStringUTF8(IDS_CONTENT_SETTINGS_FEATURES_LABEL).c_str());
Why have this label?  The dialog is obvious otherwise, and it feels out of
place.

[Nico, 2010-07-07 19:00:43]

New screenshot coming as soon as ld is done doing it's thing

http://codereview.chromium.org/2757005/diff/10001/11002
File chrome/browser/gtk/options/content_settings_window_gtk.cc (right):

http://codereview.chromium.org/2757005/diff/10001/11002#newcode28
chrome/browser/gtk/options/content_settings_window_gtk.cc:28: #include
"chrome/browser/gtk/gtk_tree.h"
On 2010/07/07 15:44:07, Evan Martin wrote:
> Why not include with the rest?

Oops :-) Done.

http://codereview.chromium.org/2757005/diff/10001/11002#newcode108
chrome/browser/gtk/options/content_settings_window_gtk.cc:108: GtkWidget* hbox =
gtk_hbox_new(FALSE, gtk_util::kControlSpacing);
On 2010/07/07 15:44:07, Evan Martin wrote:
> Can you use the wider spacing for this?

"the" wider spacing? Used one of the wider spacings.

http://codereview.chromium.org/2757005/diff/10001/11002#newcode163
chrome/browser/gtk/options/content_settings_window_gtk.cc:163:
l10n_util::GetStringUTF8(IDS_CONTENT_SETTINGS_FEATURES_LABEL).c_str());
On 2010/07/07 15:44:07, Evan Martin wrote:
> Why have this label?  The dialog is obvious otherwise, and it feels out of
> place.

'cause the mock had it :-)

Feel free to start a thread with the UI guys (I was wondering what it's there
for too), but this should probably match on windows and linux.

[Nico, 2010-07-07 19:09:12]

http://imgur.com/OISUV

[Nico, 2010-07-08 17:37:49]

Ping.

[Evan Martin, 2010-07-08 17:43:22]

LGTM

http://codereview.chromium.org/2757005/diff/14001/15002
File chrome/browser/gtk/options/content_settings_window_gtk.cc (right):

http://codereview.chromium.org/2757005/diff/14001/15002#newcode131
chrome/browser/gtk/options/content_settings_window_gtk.cc:131:
gtk_util::ShowDialogWithLocalizedSize(dialog_, -1, -1, true);
BTW, this function is completely crazy.  I kind of think "gtk_widget_show" does
what you want here, but it's probably better that you match the old code.

http://codereview.chromium.org/2757005/diff/14001/15002#newcode136
chrome/browser/gtk/options/content_settings_window_gtk.cc:136:
gtk_tree_selection_set_mode(selection, GTK_SELECTION_BROWSE);
I am happy you got this.

http://codereview.chromium.org/2757005/diff/14001/15002#newcode247
chrome/browser/gtk/options/content_settings_window_gtk.cc:247:
gtk_notebook_get_current_page(GTK_NOTEBOOK(notebook_)))
curlies, or fit on one line