gtk: fix 18949 on GTK ("Options" window does not fit a small display).

Change the layout of content and general page. Use the horizontal title and content layout for each option group.

BUG=18949
TEST=Open the option window, check the content and general pages.

Patch by Ningxin Hu <ningxin.hu@intel.com>.

Review URL: http://codereview.chromium.org/1969006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@49291 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 155 insertions, 48 deletions

diff --git a/chrome/browser/gtk/options/advanced_contents_gtk.cc b/chrome/browser/gtk/options/advanced_contents_gtk.cc
index b1257d6..5ed339d 100644
--- a/chrome/browser/gtk/options/advanced_contents_gtk.cc
+++ b/chrome/browser/gtk/options/advanced_contents_gtk.cc
@@ -1193,37 +1193,38 @@ AdvancedContentsGtk::~AdvancedContentsGtk() {
 }
 
 void AdvancedContentsGtk::Init() {
-  OptionsLayoutBuilderGtk options_builder;
+  scoped_ptr<OptionsLayoutBuilderGtk>
+    options_builder(OptionsLayoutBuilderGtk::Create());
 
   privacy_section_.reset(new PrivacySection(profile_));
-  options_builder.AddOptionGroup(
+  options_builder->AddOptionGroup(
       l10n_util::GetStringUTF8(IDS_OPTIONS_ADVANCED_SECTION_TITLE_PRIVACY),
       privacy_section_->get_page_widget(), false);
 
   network_section_.reset(new NetworkSection(profile_));
-  options_builder.AddOptionGroup(
+  options_builder->AddOptionGroup(
       l10n_util::GetStringUTF8(IDS_OPTIONS_ADVANCED_SECTION_TITLE_NETWORK),
       network_section_->get_page_widget(), false);
 
   translate_section_.reset(new TranslateSection(profile_));
-  options_builder.AddOptionGroup(
+  options_builder->AddOptionGroup(
       l10n_util::GetStringUTF8(IDS_OPTIONS_ADVANCED_SECTION_TITLE_TRANSLATE),
       translate_section_->get_page_widget(), false);
 
   download_section_.reset(new DownloadSection(profile_));
-  options_builder.AddOptionGroup(
+  options_builder->AddOptionGroup(
       l10n_util::GetStringUTF8(IDS_OPTIONS_DOWNLOADLOCATION_GROUP_NAME),
       download_section_->get_page_widget(), false);
 
   web_content_section_.reset(new WebContentSection(profile_));
-  options_builder.AddOptionGroup(
+  options_builder->AddOptionGroup(
       l10n_util::GetStringUTF8(IDS_OPTIONS_ADVANCED_SECTION_TITLE_CONTENT),
       web_content_section_->get_page_widget(), false);
 
   security_section_.reset(new SecuritySection(profile_));
-  options_builder.AddOptionGroup(
+  options_builder->AddOptionGroup(
       l10n_util::GetStringUTF8(IDS_OPTIONS_ADVANCED_SECTION_TITLE_SECURITY),
       security_section_->get_page_widget(), false);
 
-  page_ = options_builder.get_page_widget();
+  page_ = options_builder->get_page_widget();
 }
diff --git a/chrome/browser/gtk/options/content_filter_page_gtk.cc b/chrome/browser/gtk/options/content_filter_page_gtk.cc
index bf18c0c..9c2088e 100644
--- a/chrome/browser/gtk/options/content_filter_page_gtk.cc
+++ b/chrome/browser/gtk/options/content_filter_page_gtk.cc
@@ -35,11 +35,13 @@ ContentFilterPageGtk::ContentFilterPageGtk(Profile* profile,
   };
   COMPILE_ASSERT(arraysize(kTitleIDs) == CONTENT_SETTINGS_NUM_TYPES,
                  kTitleIDs_IncorrectSize);
-  OptionsLayoutBuilderGtk options_builder;
-  options_builder.AddOptionGroup(
+
+  scoped_ptr<OptionsLayoutBuilderGtk>
+    options_builder(OptionsLayoutBuilderGtk::Create());
+  options_builder->AddOptionGroup(
       l10n_util::GetStringUTF8(kTitleIDs[content_type_]),
       InitGroup(), true);
-  page_ = options_builder.get_page_widget();
+  page_ = options_builder->get_page_widget();
 }
 
 ContentFilterPageGtk::~ContentFilterPageGtk() {
diff --git a/chrome/browser/gtk/options/content_page_gtk.cc b/chrome/browser/gtk/options/content_page_gtk.cc
index 62ebcfd..4ae6ee1 100644
--- a/chrome/browser/gtk/options/content_page_gtk.cc
+++ b/chrome/browser/gtk/options/content_page_gtk.cc
@@ -87,27 +87,28 @@ ContentPageGtk::ContentPageGtk(Profile* profile)
   }
 
   // Prepare the group options layout.
-  OptionsLayoutBuilderGtk options_builder;
+  scoped_ptr<OptionsLayoutBuilderGtk>
+    options_builder(OptionsLayoutBuilderGtk::CreateOptionallyCompactLayout());
   if (sync_service_) {
-    options_builder.AddOptionGroup(
+    options_builder->AddOptionGroup(
         l10n_util::GetStringUTF8(IDS_SYNC_OPTIONS_GROUP_NAME),
         InitSyncGroup(), false);
     UpdateSyncControls();
   }
 
-  options_builder.AddOptionGroup(
+  options_builder->AddOptionGroup(
       l10n_util::GetStringUTF8(IDS_OPTIONS_PASSWORDS_GROUP_NAME),
       InitPasswordSavingGroup(), false);
-  options_builder.AddOptionGroup(
+  options_builder->AddOptionGroup(
       l10n_util::GetStringUTF8(IDS_AUTOFILL_SETTING_WINDOWS_GROUP_NAME),
       InitFormAutoFillGroup(), false);
-  options_builder.AddOptionGroup(
+  options_builder->AddOptionGroup(
       l10n_util::GetStringUTF8(IDS_OPTIONS_BROWSING_DATA_GROUP_NAME),
       InitBrowsingDataGroup(), false);
-  options_builder.AddOptionGroup(
+  options_builder->AddOptionGroup(
       l10n_util::GetStringUTF8(IDS_APPEARANCE_GROUP_NAME),
       InitThemesGroup(), false);
-  page_ = options_builder.get_page_widget();
+  page_ = options_builder->get_page_widget();
 
   // Add preferences observers.
   ask_to_save_passwords_.Init(prefs::kPasswordManagerEnabled,
diff --git a/chrome/browser/gtk/options/cookie_filter_page_gtk.cc b/chrome/browser/gtk/options/cookie_filter_page_gtk.cc
index 726c1f3..e5714b2 100644
--- a/chrome/browser/gtk/options/cookie_filter_page_gtk.cc
+++ b/chrome/browser/gtk/options/cookie_filter_page_gtk.cc
@@ -33,11 +33,12 @@ GtkWidget* WrapInHBox(GtkWidget* widget) {
 CookieFilterPageGtk::CookieFilterPageGtk(Profile* profile)
     : OptionsPageBase(profile),
       initializing_(true) {
-  OptionsLayoutBuilderGtk options_builder;
-  options_builder.AddOptionGroup(
+  scoped_ptr<OptionsLayoutBuilderGtk>
+    options_builder(OptionsLayoutBuilderGtk::Create());
+  options_builder->AddOptionGroup(
       l10n_util::GetStringUTF8(IDS_MODIFY_COOKIE_STORING_LABEL),
       InitCookieStoringGroup(), true);
-  page_ = options_builder.get_page_widget();
+  page_ = options_builder->get_page_widget();
 
   clear_site_data_on_exit_.Init(prefs::kClearSiteDataOnExit,
                                 profile->GetPrefs(), NULL);
diff --git a/chrome/browser/gtk/options/fonts_page_gtk.cc b/chrome/browser/gtk/options/fonts_page_gtk.cc
index 9e73e3e..96c918e 100644
--- a/chrome/browser/gtk/options/fonts_page_gtk.cc
+++ b/chrome/browser/gtk/options/fonts_page_gtk.cc
@@ -43,8 +43,8 @@ FontsPageGtk::~FontsPageGtk() {
 }
 
 void FontsPageGtk::Init() {
-  OptionsLayoutBuilderGtk options_builder;
-
+  scoped_ptr<OptionsLayoutBuilderGtk>
+    options_builder(OptionsLayoutBuilderGtk::Create());
   serif_font_button_ = gtk_font_button_new();
   gtk_font_button_set_use_font(GTK_FONT_BUTTON(serif_font_button_), TRUE);
   gtk_font_button_set_use_size(GTK_FONT_BUTTON(serif_font_button_), TRUE);
@@ -75,7 +75,7 @@ void FontsPageGtk::Init() {
       fixed_font_button_,
       NULL);
 
-  options_builder.AddOptionGroup(l10n_util::GetStringUTF8(
+  options_builder->AddOptionGroup(l10n_util::GetStringUTF8(
         IDS_FONT_LANGUAGE_SETTING_FONT_SUB_DIALOG_FONT_TITLE),
       font_controls, false);
 
@@ -86,11 +86,11 @@ void FontsPageGtk::Init() {
       encoding_group_description.c_str(),
       default_encoding_combobox_,
       NULL);
-  options_builder.AddOptionGroup(l10n_util::GetStringUTF8(
+  options_builder->AddOptionGroup(l10n_util::GetStringUTF8(
         IDS_FONT_LANGUAGE_SETTING_FONT_SUB_DIALOG_ENCODING_TITLE),
       encoding_controls, false);
 
-  page_ = options_builder.get_page_widget();
+  page_ = options_builder->get_page_widget();
 
   serif_name_.Init(prefs::kWebKitSerifFontFamily, profile()->GetPrefs(), this);
   sans_serif_name_.Init(prefs::kWebKitSansSerifFontFamily,
diff --git a/chrome/browser/gtk/options/general_page_gtk.cc b/chrome/browser/gtk/options/general_page_gtk.cc
index 3919232..d36fbac 100644
--- a/chrome/browser/gtk/options/general_page_gtk.cc
+++ b/chrome/browser/gtk/options/general_page_gtk.cc
@@ -75,22 +75,23 @@ GeneralPageGtk::GeneralPageGtk(Profile* profile)
           new ShellIntegration::DefaultBrowserWorker(this)),
       managed_prefs_banner_(profile->GetPrefs(), kGeneralManagablePrefs,
                             arraysize(kGeneralManagablePrefs)) {
-  OptionsLayoutBuilderGtk options_builder;
-  page_ = options_builder.get_page_widget();
+  scoped_ptr<OptionsLayoutBuilderGtk>
+    options_builder(OptionsLayoutBuilderGtk::CreateOptionallyCompactLayout());
+  page_ = options_builder->get_page_widget();
   accessible_widget_helper_.reset(new AccessibleWidgetHelper(page_, profile));
 
-  options_builder.AddWidget(managed_prefs_banner_.banner_widget(), false);
-  options_builder.AddOptionGroup(
+  options_builder->AddWidget(managed_prefs_banner_.banner_widget(), false);
+  options_builder->AddOptionGroup(
       l10n_util::GetStringUTF8(IDS_OPTIONS_STARTUP_GROUP_NAME),
       InitStartupGroup(), true);
-  options_builder.AddOptionGroup(
+  options_builder->AddOptionGroup(
       l10n_util::GetStringUTF8(IDS_OPTIONS_HOMEPAGE_GROUP_NAME),
       InitHomepageGroup(), false);
-  options_builder.AddOptionGroup(
+  options_builder->AddOptionGroup(
       l10n_util::GetStringUTF8(IDS_OPTIONS_DEFAULTSEARCH_GROUP_NAME),
       InitDefaultSearchGroup(), false);
 #if !defined(OS_CHROMEOS)
-  options_builder.AddOptionGroup(
+  options_builder->AddOptionGroup(
       l10n_util::GetStringUTF8(IDS_OPTIONS_DEFAULTBROWSER_GROUP_NAME),
       InitDefaultBrowserGroup(), false);
 #endif
diff --git a/chrome/browser/gtk/options/options_layout_gtk.cc b/chrome/browser/gtk/options/options_layout_gtk.cc
index c4a6e60..d0ae714 100644
--- a/chrome/browser/gtk/options/options_layout_gtk.cc
+++ b/chrome/browser/gtk/options/options_layout_gtk.cc
@@ -6,15 +6,32 @@
 
 #include "chrome/browser/gtk/gtk_util.h"
 
-OptionsLayoutBuilderGtk::OptionsLayoutBuilderGtk() {
+// If the height of screen is equal or shorter than this, we will use
+// a more compact option layout.
+const int kCompactScreenHeight = 600;
+
+// Default option layout builder follows GNOME HIG, which uses header and
+// spacing to group options.
+class DefaultOptionsLayoutBuilderGtk : public OptionsLayoutBuilderGtk {
+ public:
+  explicit DefaultOptionsLayoutBuilderGtk();
+
+  void AddOptionGroup(const std::string& title, GtkWidget* content,
+                      bool expandable);
+  void AddWidget(GtkWidget* content, bool expandable);
+ private:
+  DISALLOW_COPY_AND_ASSIGN(DefaultOptionsLayoutBuilderGtk);
+};
+
+DefaultOptionsLayoutBuilderGtk::DefaultOptionsLayoutBuilderGtk() {
   page_ = gtk_vbox_new(FALSE, gtk_util::kContentAreaSpacing);
   gtk_container_set_border_width(GTK_CONTAINER(page_),
                                  gtk_util::kContentAreaBorder);
 }
 
-void OptionsLayoutBuilderGtk::AddOptionGroup(const std::string& title,
-                                             GtkWidget* content,
-                                             bool expandable) {
+void DefaultOptionsLayoutBuilderGtk::AddOptionGroup(const std::string& title,
+                                                    GtkWidget* content,
+                                                    bool expandable) {
   GtkWidget* title_label = gtk_util::CreateBoldLabel(title);
 
   GtkWidget* group = gtk_vbox_new(FALSE, gtk_util::kControlSpacing);
@@ -24,6 +41,85 @@ void OptionsLayoutBuilderGtk::AddOptionGroup(const std::string& title,
   AddWidget(group, expandable);
 }
 
-void OptionsLayoutBuilderGtk::AddWidget(GtkWidget* content, bool expandable) {
+void DefaultOptionsLayoutBuilderGtk::AddWidget(GtkWidget* content,
+                                               bool expandable) {
   gtk_box_pack_start(GTK_BOX(page_), content, expandable, expandable, 0);
 }
+
+// Compact layout builder uses table to layout label and content horizontally.
+class CompactOptionsLayoutBuilderGtk : public OptionsLayoutBuilderGtk {
+ public:
+  explicit CompactOptionsLayoutBuilderGtk();
+
+  void AddOptionGroup(const std::string& title, GtkWidget* content,
+                      bool expandable);
+  void AddWidget(GtkWidget* content, bool expandable);
+ private:
+  GtkWidget *table_;
+  guint row_;
+
+  DISALLOW_COPY_AND_ASSIGN(CompactOptionsLayoutBuilderGtk);
+};
+
+CompactOptionsLayoutBuilderGtk::CompactOptionsLayoutBuilderGtk() {
+  row_ = 0;
+  table_ = NULL;
+
+  page_ = gtk_vbox_new(FALSE, gtk_util::kContentAreaSpacing);
+  gtk_container_set_border_width(GTK_CONTAINER(page_),
+                                 gtk_util::kContentAreaBorder);
+}
+
+void CompactOptionsLayoutBuilderGtk::AddOptionGroup(const std::string& title,
+                                             GtkWidget* content,
+                                             bool expandable) {
+  if (!table_) {
+    // Create a new table to contain option groups
+    table_ = gtk_table_new(0, 2, FALSE);
+    gtk_table_set_col_spacing(GTK_TABLE(table_), 0, gtk_util::kLabelSpacing);
+    gtk_table_set_row_spacings(GTK_TABLE(table_),
+                               gtk_util::kContentAreaSpacing);
+
+    gtk_container_set_border_width(GTK_CONTAINER(table_),
+                                   gtk_util::kContentAreaBorder);
+    gtk_box_pack_start(GTK_BOX(page_), table_, TRUE, TRUE, 0);
+  }
+
+  GtkWidget* title_label = gtk_util::CreateBoldLabel(title);
+
+  gtk_table_resize(GTK_TABLE(table_), row_ + 1, 2);
+  gtk_misc_set_alignment(GTK_MISC(title_label), 1, 0);
+
+  gtk_table_attach(GTK_TABLE(table_), title_label,
+                   0, 1, row_, row_ + 1,
+                   GTK_FILL, GTK_FILL,
+                   0, 0);
+  gtk_table_attach(GTK_TABLE(table_), content,
+                   1, 2, row_, row_ + 1,
+                   expandable ?
+                   GTK_FILL : GtkAttachOptions(GTK_FILL | GTK_EXPAND),
+                   GTK_FILL,
+                   0, 0);
+  row_++;
+}
+
+void CompactOptionsLayoutBuilderGtk::AddWidget(GtkWidget* content,
+                                               bool expandable) {
+  gtk_box_pack_start(GTK_BOX(page_), content, expandable, expandable, 0);
+
+  // Let AddOptionGroup create a new table and append after this widget
+  table_ = NULL;
+}
+
+OptionsLayoutBuilderGtk* OptionsLayoutBuilderGtk::Create() {
+    return new DefaultOptionsLayoutBuilderGtk();
+}
+
+OptionsLayoutBuilderGtk*
+OptionsLayoutBuilderGtk::CreateOptionallyCompactLayout() {
+  gint screen_height = gdk_screen_get_height(gdk_screen_get_default());
+  if (screen_height <= kCompactScreenHeight)
+    return new CompactOptionsLayoutBuilderGtk();
+  else
+    return new DefaultOptionsLayoutBuilderGtk();
+}
diff --git a/chrome/browser/gtk/options/options_layout_gtk.h b/chrome/browser/gtk/options/options_layout_gtk.h
index 0d397c0..96bbc8ba 100644
--- a/chrome/browser/gtk/options/options_layout_gtk.h
+++ b/chrome/browser/gtk/options/options_layout_gtk.h
@@ -12,28 +12,33 @@
 
 class OptionsLayoutBuilderGtk {
  public:
-  explicit OptionsLayoutBuilderGtk();
-
   GtkWidget* get_page_widget() {
     return page_;
   }
 
-  // Adds an option group to the table.  Handles layout and labels the group
-  // with the given title.  If expandable is true, the content widget will be
-  // allowed to expand and fill any extra space when the dialog is resized.
-  void AddOptionGroup(const std::string& title, GtkWidget* content,
-                      bool expandable);
+  // Adds an option group to the table.  Handles layout and the placing of
+  // separators between groups.  If expandable is true, the content widget will
+  // be allowed to expand and fill any extra space when the dialog is resized.
+  virtual void AddOptionGroup(const std::string& title, GtkWidget* content,
+                              bool expandable) = 0;
 
   // Adds a widget without title or special layout.  If expandable is true, the
   // content widget will be allowed to expand and fill any extra space when the
   // dialog is resized.
-  void AddWidget(GtkWidget* content, bool expandable);
+  virtual void AddWidget(GtkWidget* content, bool expandable) = 0;
+
+  // Creates a default option layout builder. The default layout builder
+  // follows the GNOME HIG.
+  static OptionsLayoutBuilderGtk* Create();
+
+  // Creates a compact option layout builder, if the screen is compact.
+  // Otherwise, creates a default one.
+  static OptionsLayoutBuilderGtk* CreateOptionallyCompactLayout();
 
- private:
+ protected:
   // The parent widget
   GtkWidget* page_;
 
-  DISALLOW_COPY_AND_ASSIGN(OptionsLayoutBuilderGtk);
 };
 
 #endif  // CHROME_BROWSER_GTK_OPTIONS_OPTIONS_LAYOUT_GTK_H_