Skip to content

Commit

Permalink
Invalid URLs are no longer mangled when reopening the Options window.
Browse files Browse the repository at this point in the history
If the homepage URL is changed to an invalid URL, the homepage preference is swapped to a blank URL and NTP is shown.

BUG=40996
TEST=Set homepage to http://www.google.com, close Chromium, restart Chromium and see homepage set to http://www.google.com. Set homepage to http://, close Chromium, restart Chromium and see homepage set to New Tab Page.

Patch by Jared Wein <weinjared@gmail.com>

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@48270 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
mhm@chromium.org committed May 26, 2010
1 parent 78ff4cd commit 587325e
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 30 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,4 @@ Ryan Sleevi <ryan.sleevi@gmail.com>
Satoshi Matsuzaki <satoshi.matsuzaki@gmail.com>
Benjamin Jemlich <pcgod99@gmail.com>
Ningxin Hu <ningxin.hu@intel.com>
Jared Wein <weinjared@gmail.com>
23 changes: 11 additions & 12 deletions chrome/browser/cocoa/preferences_window_controller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -796,18 +796,19 @@ - (void)launchKeychainAccess {
// Basics panel

// Sets the home page preferences for kNewTabPageIsHomePage and kHomePage. If a
// blank string is passed in we revert to using NewTab page as the Home page.
// When setting the Home Page to NewTab page, we preserve the old value of
// kHomePage (we don't overwrite it). Note: using SetValue() causes the
// observers not to fire, which is actually a good thing as we could end up in a
// state where setting the homepage to an empty url would automatically reset
// the prefs back to using the NTP, so we'd be never be able to change it.
- (void)setHomepage:(const std::string&)homepage {
if (homepage.empty() || homepage == GetNewTabUIURLString()) {
// blank or null-host URL is passed in we revert to using NewTab page
// as the Home page. Note: using SetValue() causes the observers not to fire,
// which is actually a good thing as we could end up in a state where setting
// the homepage to an empty url would automatically reset the prefs back to
// using the NTP, so we'd be never be able to change it.
- (void)setHomepage:(const GURL&)homepage {
if (!homepage.is_valid() || homepage.spec() == GetNewTabUIURLString()) {
newTabPageIsHomePage_.SetValue(true);
if (!homepage.has_host())
homepage_.SetValue(std::wstring());
} else {
newTabPageIsHomePage_.SetValue(false);
homepage_.SetValue(UTF8ToWide(homepage));
homepage_.SetValue(UTF8ToWide(homepage.spec()));
}
}

Expand Down Expand Up @@ -1013,9 +1014,7 @@ - (void)setHomepageURL:(NSString*)urlString {
// to something valid ("http://google.com").
std::string unfixedURL = urlString ? base::SysNSStringToUTF8(urlString) :
chrome::kChromeUINewTabURL;
std::string fixedURL = URLFixerUpper::FixupURL(unfixedURL, std::string());
if (GURL(fixedURL).is_valid())
[self setHomepage:fixedURL];
[self setHomepage:GURL(URLFixerUpper::FixupURL(unfixedURL, std::string()))];
}

// Returns whether the home button should be checked based on the preference.
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/gtk/options/general_page_gtk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,8 @@ void GeneralPageGtk::EnableDefaultSearchEngineComboBox(bool enable) {
void GeneralPageGtk::SetHomepage(const GURL& homepage) {
if (!homepage.is_valid() || homepage.spec() == chrome::kChromeUINewTabURL) {
new_tab_page_is_home_page_.SetValue(true);
if (!homepage.has_host())
homepage_.SetValue(std::wstring());
} else {
new_tab_page_is_home_page_.SetValue(false);
homepage_.SetValue(UTF8ToWide(homepage.spec()));
Expand Down
5 changes: 2 additions & 3 deletions chrome/browser/gtk/options/general_page_gtk.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,8 @@ class GeneralPageGtk : public OptionsPageBase,
void EnableDefaultSearchEngineComboBox(bool enable);

// Sets the home page preferences for kNewTabPageIsHomePage and kHomePage.
// If a blank string is passed in we revert to using NewTab page as the Home
// page. When setting the Home Page to NewTab page, we preserve the old value
// of kHomePage (we don't overwrite it).
// If an invalid URL is passed in we revert to using NewTab page as the Home
// page.
void SetHomepage(const GURL& homepage);

// Sets the home page pref using the value in the entry box
Expand Down
22 changes: 12 additions & 10 deletions chrome/browser/views/options/general_page_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -232,12 +232,12 @@ void GeneralPageView::ButtonPressed(
} else if (sender == homepage_use_newtab_radio_) {
UserMetricsRecordAction(UserMetricsAction("Options_Homepage_UseNewTab"),
profile()->GetPrefs());
SetHomepage(GetNewTabUIURLString());
SetHomepage(GURL());
EnableHomepageURLField(false);
} else if (sender == homepage_use_url_radio_) {
UserMetricsRecordAction(UserMetricsAction("Options_Homepage_UseURL"),
profile()->GetPrefs());
SetHomepage(homepage_use_url_textfield_->text());
SetHomepage(GURL(homepage_use_url_textfield_->text()));
EnableHomepageURLField(true);
} else if (sender == homepage_show_home_button_checkbox_) {
bool show_button = homepage_show_home_button_checkbox_->checked();
Expand Down Expand Up @@ -285,11 +285,10 @@ void GeneralPageView::ContentsChanged(views::Textfield* sender,
if (sender == homepage_use_url_textfield_) {
// If the text field contains a valid URL, sync it to prefs. We run it
// through the fixer upper to allow input like "google.com" to be converted
// to something valid ("http://google.com").
std::string url_string = URLFixerUpper::FixupURL(
UTF16ToUTF8(homepage_use_url_textfield_->text()), std::string());
if (GURL(url_string).is_valid())
SetHomepage(UTF8ToWide(url_string));
// to something valid ("http://google.com"). If the field contains an
// empty or null-host URL, a blank homepage is synced to prefs.
SetHomepage(GURL(URLFixerUpper::FixupURL(
UTF16ToUTF8(homepage_use_url_textfield_->text()), std::string())));
}
}

Expand Down Expand Up @@ -745,12 +744,15 @@ void GeneralPageView::AddBookmark(UrlPicker* dialog,
SaveStartupPref();
}

void GeneralPageView::SetHomepage(const std::wstring& homepage) {
if (homepage.empty() || homepage == GetNewTabUIURLString()) {
void GeneralPageView::SetHomepage(const GURL& homepage) {
if (!homepage.is_valid() ||
UTF8ToWide(homepage.spec()) == GetNewTabUIURLString()) {
new_tab_page_is_home_page_.SetValue(true);
if (!homepage.has_host())
homepage_.SetValue(std::wstring());
} else {
new_tab_page_is_home_page_.SetValue(false);
homepage_.SetValue(homepage);
homepage_.SetValue(UTF8ToWide(homepage.spec()));
}
}

Expand Down
9 changes: 4 additions & 5 deletions chrome/browser/views/options/general_page_view.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2009 The Chromium Authors. All rights reserved.
// Copyright (c) 2010 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

Expand Down Expand Up @@ -105,10 +105,9 @@ class GeneralPageView : public OptionsPageView,
const GURL& url);

// Sets the home page preferences for kNewTabPageIsHomePage and kHomePage.
// If a blank string is passed in we revert to using NewTab page as the Home
// page. When setting the Home Page to NewTab page, we preserve the old value
// of kHomePage (we don't overwrite it).
void SetHomepage(const std::wstring& homepage);
// If an empty or null-host GURL is passed in we revert to using NewTab
// page as the Homepage.
void SetHomepage(const GURL& homepage);

// Invoked when the selection of the table view changes. Updates the enabled
// property of the remove button.
Expand Down

0 comments on commit 587325e

Please sign in to comment.