diff options
author | csilv@chromium.org <csilv@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-11-22 02:59:01 +0000 |
---|---|---|
committer | csilv@chromium.org <csilv@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-11-22 02:59:01 +0000 |
commit | 0f083403ad489e6fd3305c540e14ea6c9a460061 (patch) | |
tree | aab6a4a6772cc76c6389aa45c64b4339398365e9 | |
parent | 8caadebcb838083821519861da751992401ce0fe (diff) | |
download | chromium_src-0f083403ad489e6fd3305c540e14ea6c9a460061.zip chromium_src-0f083403ad489e6fd3305c540e14ea6c9a460061.tar.gz chromium_src-0f083403ad489e6fd3305c540e14ea6c9a460061.tar.bz2 |
Page zoom improvements:
1. Change from using zoom levels to a range of preset zoom factors (percentages). Zoom levels are the values that WebKit uses for page zoom. While zoom levels are simple to implement, they result in undesirable percentages like 57%, 144%, 207%, etc. Changing to zoom factors gives us user-friendly zoom percentages.
2. Increase the range of supported zoom values. A user can now zoom between 25% to 500%.
3. Use an epsilon value when comparing zoom floating point values. The previous version was doing an equality comparison of double values which may not always work as expected.
BUG=71484
TEST=Exercise zoom in/out via the wrench menu; exercise default page zoom setting in options window
Review URL: http://codereview.chromium.org/8528011
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@111087 0039d316-1c4b-4281-b951-d872f2087c98
24 files changed, 605 insertions, 48 deletions
diff --git a/chrome/browser/chrome_page_zoom.cc b/chrome/browser/chrome_page_zoom.cc new file mode 100644 index 0000000..638ad50 --- /dev/null +++ b/chrome/browser/chrome_page_zoom.cc @@ -0,0 +1,60 @@ +// Copyright (c) 2011 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. + +#include <cmath> + +#include "chrome/browser/chrome_page_zoom.h" +#include "content/public/common/page_zoom.h" +#include "third_party/WebKit/Source/WebKit/chromium/public/WebView.h" + +namespace chrome_page_zoom { + +enum PageZoomValueType { + PAGE_ZOOM_VALUE_TYPE_FACTOR, + PAGE_ZOOM_VALUE_TYPE_LEVEL, +}; + +const double kPresetZoomFactors[] = { 0.25, 0.333, 0.5, 0.666, 0.75, 0.9, 1.0, + 1.1, 1.25, 1.5, 1.75, 2.0, 2.5, 3.0, 4.0, + 5.0 }; + +std::vector<double> PresetZoomValues(PageZoomValueType value_type, + double custom_value) { + // Generate a vector of zoom values from an array of known preset + // factors. The values in content::kPresetZoomFactors will already be in + // sorted order. + std::vector<double> zoom_values; + bool found_custom = false; + for (size_t i = 0; i < arraysize(kPresetZoomFactors); i++) { + double zoom_value = kPresetZoomFactors[i]; + if (value_type == PAGE_ZOOM_VALUE_TYPE_LEVEL) + zoom_value = WebKit::WebView::zoomFactorToZoomLevel(zoom_value); + if (content::ZoomValuesEqual(zoom_value, custom_value)) + found_custom = true; + zoom_values.push_back(zoom_value); + } + // If the preset array did not contain the custom value, append it to the + // vector and then sort. + double min = value_type == PAGE_ZOOM_VALUE_TYPE_LEVEL ? + WebKit::WebView::zoomFactorToZoomLevel(content::kMinimumZoomFactor) : + content::kMinimumZoomFactor; + double max = value_type == PAGE_ZOOM_VALUE_TYPE_LEVEL ? + WebKit::WebView::zoomFactorToZoomLevel(content::kMaximumZoomFactor) : + content::kMaximumZoomFactor; + if (!found_custom && custom_value > min && custom_value < max) { + zoom_values.push_back(custom_value); + std::sort(zoom_values.begin(), zoom_values.end()); + } + return zoom_values; +} + +std::vector<double> PresetZoomFactors(double custom_factor) { + return PresetZoomValues(PAGE_ZOOM_VALUE_TYPE_FACTOR, custom_factor); +} + +std::vector<double> PresetZoomLevels(double custom_level) { + return PresetZoomValues(PAGE_ZOOM_VALUE_TYPE_LEVEL, custom_level); +} + +} // namespace chrome_page_zoom diff --git a/chrome/browser/chrome_page_zoom.h b/chrome/browser/chrome_page_zoom.h new file mode 100644 index 0000000..f45dc8c --- /dev/null +++ b/chrome/browser/chrome_page_zoom.h @@ -0,0 +1,25 @@ +// Copyright (c) 2011 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. + +#ifndef CHROME_BROWSER_CHROME_PAGE_ZOOM_H_ +#define CHROME_BROWSER_CHROME_PAGE_ZOOM_H_ +#pragma once + +#include <vector> + +namespace chrome_page_zoom { + +// Return a sorted vector of zoom factors. The vector will consist of preset +// values along with a custom value (if the custom value is not already +// represented.) +std::vector<double> PresetZoomFactors(double custom_factor); + +// Return a sorted vector of zoom levels. The vector will consist of preset +// values along with a custom value (if the custom value is not already +// represented.) +std::vector<double> PresetZoomLevels(double custom_level); + +} // namespace chrome_page_zoom + +#endif // CHROME_BROWSER_CHROME_PAGE_ZOOM_H_ diff --git a/chrome/browser/chrome_page_zoom_unittest.cc b/chrome/browser/chrome_page_zoom_unittest.cc new file mode 100644 index 0000000..5784819 --- /dev/null +++ b/chrome/browser/chrome_page_zoom_unittest.cc @@ -0,0 +1,97 @@ +// Copyright (c) 2011 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. + +#include "chrome/browser/chrome_page_zoom.h" +#include "content/public/common/page_zoom.h" +#include "testing/gtest/include/gtest/gtest.h" + +TEST(ChromePageZoomTest, PresetZoomFactors) { + // Fetch a vector of preset zoom factors, including a custom value that we + // already know is not going to be in the list. + double custom_value = 1.05; // 105% + std::vector<double> factors = + chrome_page_zoom::PresetZoomFactors(custom_value); + + // Expect at least 10 zoom factors. + EXPECT_GE(factors.size(), 10U); + + // Expect the first and last items to match the minimum and maximum values. + EXPECT_DOUBLE_EQ(factors.front(), content::kMinimumZoomFactor); + EXPECT_DOUBLE_EQ(factors.back(), content::kMaximumZoomFactor); + + // Iterate through the vector, with the following checks: + // 1. The values are in sorted order. + // 2. The custom value is exists. + // 3. The 100% value exists. + bool found_custom_value = false; + bool found_100_percent = false; + double last_value = 0; + + std::vector<double>::const_iterator i; + for (i = factors.begin(); i != factors.end(); ++i) { + double factor = *i; + EXPECT_GT(factor, last_value); + if (content::ZoomValuesEqual(factor, custom_value)) + found_custom_value = true; + if (content::ZoomValuesEqual(factor, 1.0)) + found_100_percent = true; + last_value = factor; + } + + EXPECT_TRUE(found_custom_value); + EXPECT_TRUE(found_100_percent); +} + +TEST(ChromePageZoomTest, PresetZoomLevels) { + // Fetch a vector of preset zoom levels, including a custom value that we + // already know is not going to be in the list. + double custom_value = 0.1; + std::vector<double> levels = chrome_page_zoom::PresetZoomLevels(custom_value); + + // Expect at least 10 zoom levels. + EXPECT_GE(levels.size(), 10U); + + // Iterate through the vector, with the following checks: + // 1. The values are in sorted order. + // 2. The custom value is exists. + // 3. The 100% value exists. + bool found_custom_value = false; + bool found_100_percent = false; + double last_value = -99; + + std::vector<double>::const_iterator i; + for (i = levels.begin(); i != levels.end(); ++i) { + double level = *i; + EXPECT_GT(level, last_value); + if (content::ZoomValuesEqual(level, custom_value)) + found_custom_value = true; + if (content::ZoomValuesEqual(level, 0)) + found_100_percent = true; + last_value = level; + } + + EXPECT_TRUE(found_custom_value); + EXPECT_TRUE(found_100_percent); +} + +TEST(ChromePageZoomTest, InvalidCustomFactor) { + double too_low = 0.01; + std::vector<double> factors = chrome_page_zoom::PresetZoomFactors(too_low); + EXPECT_FALSE(content::ZoomValuesEqual(factors.front(), too_low)); + + double too_high = 99.0; + factors = chrome_page_zoom::PresetZoomFactors(too_high); + EXPECT_FALSE(content::ZoomValuesEqual(factors.back(), too_high)); +} + +TEST(ChromePageZoomTest, InvalidCustomLevel) { + double too_low = -99.0; + std::vector<double> levels = chrome_page_zoom::PresetZoomLevels(too_low); + EXPECT_FALSE(content::ZoomValuesEqual(levels.front(), too_low)); + + double too_high = 99.0; + levels = chrome_page_zoom::PresetZoomLevels(too_high); + EXPECT_FALSE(content::ZoomValuesEqual(levels.back(), too_high)); +} + diff --git a/chrome/browser/resources/options/advanced_options.html b/chrome/browser/resources/options/advanced_options.html index e2cd064..cfc653a 100644 --- a/chrome/browser/resources/options/advanced_options.html +++ b/chrome/browser/resources/options/advanced_options.html @@ -101,20 +101,8 @@ </div> <div class="section-group"> <label class="web-content-select-label"> - <span i18n-content="defaultZoomLevelLabel"></span> - <select id="defaultZoomLevel" pref="profile.default_zoom_level" - data-type="double" metric="Options_ChangeDefaultZoomLevel"> - <option value="-3">57%</option> - <option value="-2">69%</option> - <option value="-1">83%</option> - <option value="0">100%</option> - <option value="1">120%</option> - <option value="2">144%</option> - <option value="3">172%</option> - <option value="4">207%</option> - <option value="5">248%</option> - <option value="6">298%</option> - </select> + <span i18n-content="defaultZoomFactorLabel"></span> + <select id="defaultZoomFactor" dataType="double"></select> </label> </div> <if expr="not pp_ifdef('chromeos') or os == 'win32'"> diff --git a/chrome/browser/resources/options/advanced_options.js b/chrome/browser/resources/options/advanced_options.js index d3bfbc6..fb1a809 100644 --- a/chrome/browser/resources/options/advanced_options.js +++ b/chrome/browser/resources/options/advanced_options.js @@ -63,6 +63,11 @@ var OptionsPage = options.OptionsPage; chrome.send('defaultFontSizeAction', [String(event.target.options[event.target.selectedIndex].value)]); }; + $('defaultZoomFactor').onchange = function(event) { + chrome.send('defaultZoomFactorAction', + [String(event.target.options[event.target.selectedIndex].value)]); + }; + $('language-button').onclick = function(event) { OptionsPage.navigateToPage('languages'); chrome.send('coreOptionsUserMetricsAction', @@ -171,6 +176,30 @@ var OptionsPage = options.OptionsPage; $('Custom').selected = true; }; + /** + * Populate the page zoom selector with values received from the caller. + * @param {Array} items An array of items to populate the selector. + * each object is an array with three elements as follows: + * 0: The title of the item (string). + * 1: The value of the item (number). + * 2: Whether the item should be selected (boolean). + */ + AdvancedOptions.SetupPageZoomSelector = function(items) { + var element = $('defaultZoomFactor'); + + // Remove any existing content. + element.textContent = ''; + + // Insert new child nodes into select element. + var value, title, selected; + for (var i = 0; i < items.length; i++) { + title = items[i][0]; + value = items[i][1]; + selected = items[i][2]; + element.appendChild(new Option(title, value, false, selected)); + } + }; + // Set the enabled state for the autoOpenFileTypesResetToDefault button. AdvancedOptions.SetAutoOpenFileTypesDisabledAttribute = function(disabled) { if (!cr.isChromeOS) { diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc index 4424b69..b2416e9 100644 --- a/chrome/browser/ui/browser.cc +++ b/chrome/browser/ui/browser.cc @@ -33,6 +33,7 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/browser_shutdown.h" #include "chrome/browser/character_encoding.h" +#include "chrome/browser/chrome_page_zoom.h" #include "chrome/browser/content_settings/host_content_settings_map.h" #include "chrome/browser/custom_handlers/protocol_handler_registry.h" #include "chrome/browser/custom_handlers/register_protocol_handler_infobar_delegate.h" @@ -152,6 +153,7 @@ #include "content/public/browser/notification_details.h" #include "content/public/common/content_restriction.h" #include "content/public/common/content_switches.h" +#include "content/public/common/page_zoom.h" #include "grit/chromium_strings.h" #include "grit/generated_resources.h" #include "grit/locale_settings.h" @@ -1900,16 +1902,53 @@ void Browser::Zoom(content::PageZoom zoom) { if (is_devtools()) return; - static const UserMetricsAction kActions[] = { - UserMetricsAction("ZoomMinus"), - UserMetricsAction("ZoomNormal"), - UserMetricsAction("ZoomPlus") - }; + RenderViewHost* host = GetSelectedTabContentsWrapper()->render_view_host(); + if (zoom == content::PAGE_ZOOM_RESET) { + host->SetZoomLevel(0); + UserMetrics::RecordAction(UserMetricsAction("ZoomNormal")); + return; + } - UserMetrics::RecordAction(kActions[zoom - content::PAGE_ZOOM_OUT]); - TabContentsWrapper* tab_contents = GetSelectedTabContentsWrapper(); - RenderViewHost* host = tab_contents->render_view_host(); - host->Zoom(zoom); + double current_zoom_level = GetSelectedTabContents()->GetZoomLevel(); + double default_zoom_level = + profile_->GetPrefs()->GetDouble(prefs::kDefaultZoomLevel); + + // Generate a vector of zoom levels from an array of known presets along with + // the default level added if necessary. + std::vector<double> zoom_levels = + chrome_page_zoom::PresetZoomLevels(default_zoom_level); + + if (zoom == content::PAGE_ZOOM_OUT) { + // Iterate through the zoom levels in reverse order to find the next + // lower level based on the current zoom level for this page. + for (std::vector<double>::reverse_iterator i = zoom_levels.rbegin(); + i != zoom_levels.rend(); ++i) { + double zoom_level = *i; + if (content::ZoomValuesEqual(zoom_level, current_zoom_level)) + continue; + if (zoom_level < current_zoom_level) { + host->SetZoomLevel(zoom_level); + UserMetrics::RecordAction(UserMetricsAction("ZoomMinus")); + return; + } + UserMetrics::RecordAction(UserMetricsAction("ZoomMinus_AtMinimum")); + } + } else { + // Iterate through the zoom levels in normal order to find the next + // higher level based on the current zoom level for this page. + for (std::vector<double>::const_iterator i = zoom_levels.begin(); + i != zoom_levels.end(); ++i) { + double zoom_level = *i; + if (content::ZoomValuesEqual(zoom_level, current_zoom_level)) + continue; + if (zoom_level > current_zoom_level) { + host->SetZoomLevel(zoom_level); + UserMetrics::RecordAction(UserMetricsAction("ZoomPlus")); + return; + } + } + UserMetrics::RecordAction(UserMetricsAction("ZoomPlus_AtMaximum")); + } } void Browser::FocusToolbar() { diff --git a/chrome/browser/ui/browser_browsertest.cc b/chrome/browser/ui/browser_browsertest.cc index 53bdf31..aec6c44 100644 --- a/chrome/browser/ui/browser_browsertest.cc +++ b/chrome/browser/ui/browser_browsertest.cc @@ -1286,6 +1286,40 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, EXPECT_FALSE(command_updater->IsCommandEnabled(IDC_IMPORT_SETTINGS)); } +IN_PROC_BROWSER_TEST_F(BrowserTest, PageZoom) { + TabContents* contents = browser()->GetSelectedTabContents(); + bool enable_plus, enable_minus; + + ui_test_utils::WindowedNotificationObserver zoom_in_observer( + content::NOTIFICATION_ZOOM_LEVEL_CHANGED, + content::NotificationService::AllSources()); + browser()->Zoom(content::PAGE_ZOOM_IN); + zoom_in_observer.Wait(); + EXPECT_EQ(contents->GetZoomPercent(&enable_plus, &enable_minus), 110); + EXPECT_TRUE(enable_plus); + EXPECT_TRUE(enable_minus); + + ui_test_utils::WindowedNotificationObserver zoom_reset_observer( + content::NOTIFICATION_ZOOM_LEVEL_CHANGED, + content::NotificationService::AllSources()); + browser()->Zoom(content::PAGE_ZOOM_RESET); + zoom_reset_observer.Wait(); + EXPECT_EQ(contents->GetZoomPercent(&enable_plus, &enable_minus), 100); + EXPECT_TRUE(enable_plus); + EXPECT_TRUE(enable_minus); + + ui_test_utils::WindowedNotificationObserver zoom_out_observer( + content::NOTIFICATION_ZOOM_LEVEL_CHANGED, + content::NotificationService::AllSources()); + browser()->Zoom(content::PAGE_ZOOM_OUT); + zoom_out_observer.Wait(); + EXPECT_EQ(contents->GetZoomPercent(&enable_plus, &enable_minus), 90); + EXPECT_TRUE(enable_plus); + EXPECT_TRUE(enable_minus); + + browser()->Zoom(content::PAGE_ZOOM_RESET); +} + // TODO(ben): this test was never enabled. It has bit-rotted since being added. // It originally lived in browser_unittest.cc, but has been moved here to make // room for real browser unit tests. diff --git a/chrome/browser/ui/webui/options/advanced_options_browsertest.js b/chrome/browser/ui/webui/options/advanced_options_browsertest.js index a97945b..a929bbb 100644 --- a/chrome/browser/ui/webui/options/advanced_options_browsertest.js +++ b/chrome/browser/ui/webui/options/advanced_options_browsertest.js @@ -16,9 +16,48 @@ AdvancedOptionsWebUITest.prototype = { * Browse to advanced options. **/ browsePreload: 'chrome://settings/advanced', + + /** + * Register a mock handler. + * @type {Function} + * @override + */ + preLoad: function() { + this.makeAndRegisterMockHandler(['defaultZoomFactorAction']); + }, }; -// Test opening the advanced options has correct location. +/** + * The expected minimum length of the |defaultZoomFactor| element. + * @type {number} + * @const + */ +var defaultZoomFactorMinimumLength = 10; + +/** + * Test opening the advanced options has correct location. + */ TEST_F('AdvancedOptionsWebUITest', 'testOpenAdvancedOptions', function() { assertEquals(this.browsePreload, document.location.href); }); + +/** + * Test the default zoom factor select element. + */ +TEST_F('AdvancedOptionsWebUITest', 'testDefaultZoomFactor', function() { + // Verify that the zoom factor element exists. + var defaultZoomFactor = $('defaultZoomFactor'); + assertNotEquals(defaultZoomFactor, null); + + // Verify that the zoom factor element has a reasonable number of choices. + expectGE(defaultZoomFactor.options.length, defaultZoomFactorMinimumLength); + + // Simulate a change event, selecting the highest zoom value. Verify that + // the javascript handler was invoked once. + this.mockHandler.expects(once()).defaultZoomFactorAction(NOT_NULL). + will(callFunction(function() { })); + defaultZoomFactor.selectedIndex = defaultZoomFactor.options.length - 1; + var event = { target: defaultZoomFactor }; + if (defaultZoomFactor.onchange) defaultZoomFactor.onchange(event); +}); + diff --git a/chrome/browser/ui/webui/options/advanced_options_handler.cc b/chrome/browser/ui/webui/options/advanced_options_handler.cc index ed5de40..0ff6ed2 100644 --- a/chrome/browser/ui/webui/options/advanced_options_handler.cc +++ b/chrome/browser/ui/webui/options/advanced_options_handler.cc @@ -13,6 +13,7 @@ #include "base/utf_string_conversions.h" #include "base/values.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/chrome_page_zoom.h" #include "chrome/browser/download/download_prefs.h" #include "chrome/browser/google/google_util.h" #include "chrome/browser/prefs/pref_service.h" @@ -33,9 +34,11 @@ #include "content/browser/user_metrics.h" #include "content/public/browser/notification_details.h" #include "content/public/browser/notification_types.h" +#include "content/public/common/page_zoom.h" #include "grit/chromium_strings.h" #include "grit/generated_resources.h" #include "grit/locale_settings.h" +#include "third_party/WebKit/Source/WebKit/chromium/public/WebView.h" #include "ui/base/l10n/l10n_util.h" #if !defined(OS_CHROMEOS) @@ -118,7 +121,7 @@ void AdvancedOptionsHandler::GetLocalizedValues( IDS_OPTIONS_TABS_TO_LINKS_PREF }, { "fontSettingsInfo", IDS_OPTIONS_FONTSETTINGS_INFO }, - { "defaultZoomLevelLabel", + { "defaultZoomFactorLabel", IDS_OPTIONS_DEFAULT_ZOOM_LEVEL_LABEL }, { "defaultFontSizeLabel", IDS_OPTIONS_DEFAULT_FONT_SIZE_LABEL }, @@ -205,7 +208,8 @@ void AdvancedOptionsHandler::Initialize() { DCHECK(web_ui_); SetupMetricsReportingCheckbox(); SetupMetricsReportingSettingVisibility(); - SetupFontSizeLabel(); + SetupFontSizeSelector(); + SetupPageZoomSelector(); SetupAutoOpenFileTypesDisabledAttribute(); SetupProxySettingsSection(); SetupSSLConfigSettings(); @@ -249,6 +253,7 @@ WebUIMessageHandler* AdvancedOptionsHandler::Attach(WebUI* web_ui) { auto_open_files_.Init(prefs::kDownloadExtensionsToOpen, prefs, this); default_font_size_.Init(prefs::kWebKitDefaultFontSize, prefs, this); + default_zoom_level_.Init(prefs::kDefaultZoomLevel, prefs, this); #if !defined(OS_CHROMEOS) proxy_prefs_.reset( PrefSetObserver::CreateProxyPrefSetObserver(prefs, this)); @@ -270,6 +275,9 @@ void AdvancedOptionsHandler::RegisterMessages() { web_ui_->RegisterMessageCallback("defaultFontSizeAction", base::Bind(&AdvancedOptionsHandler::HandleDefaultFontSize, base::Unretained(this))); + web_ui_->RegisterMessageCallback("defaultZoomFactorAction", + base::Bind(&AdvancedOptionsHandler::HandleDefaultZoomFactor, + base::Unretained(this))); #if !defined(OS_CHROMEOS) web_ui_->RegisterMessageCallback("metricsReportingCheckboxAction", base::Bind(&AdvancedOptionsHandler::HandleMetricsReportingCheckbox, @@ -325,7 +333,9 @@ void AdvancedOptionsHandler::Observe( SetupCloudPrintProxySection(); #endif } else if (*pref_name == prefs::kWebKitDefaultFontSize) { - SetupFontSizeLabel(); + SetupFontSizeSelector(); + } else if (*pref_name == prefs::kDefaultZoomLevel) { + SetupPageZoomSelector(); #if !defined(OS_MACOSX) && !defined(OS_CHROMEOS) } else if (*pref_name == prefs::kBackgroundModeEnabled) { SetupBackgroundModeSettings(); @@ -388,11 +398,19 @@ void AdvancedOptionsHandler::HandleDefaultFontSize(const ListValue* args) { if (ExtractIntegerValue(args, &font_size)) { if (font_size > 0) { default_font_size_.SetValue(font_size); - SetupFontSizeLabel(); + SetupFontSizeSelector(); } } } +void AdvancedOptionsHandler::HandleDefaultZoomFactor(const ListValue* args) { + double zoom_factor; + if (ExtractDoubleValue(args, &zoom_factor)) { + default_zoom_level_.SetValue( + WebKit::WebView::zoomFactorToZoomLevel(zoom_factor)); + } +} + void AdvancedOptionsHandler::HandleCheckRevocationCheckbox( const ListValue* args) { std::string checked_str = UTF16ToUTF8(ExtractStringValue(args)); @@ -465,7 +483,6 @@ void AdvancedOptionsHandler::HandleDisableCloudPrintProxy( } void AdvancedOptionsHandler::RefreshCloudPrintStatusFromService() { - DCHECK(web_ui_); if (cloud_print_proxy_ui_enabled_) CloudPrintProxyServiceFactory::GetForProfile(Profile::FromWebUI(web_ui_))-> RefreshStatusFromService(); @@ -535,13 +552,48 @@ void AdvancedOptionsHandler::SetupMetricsReportingSettingVisibility() { #endif } -void AdvancedOptionsHandler::SetupFontSizeLabel() { +void AdvancedOptionsHandler::SetupFontSizeSelector() { // We're only interested in integer values, so convert to int. base::FundamentalValue font_size(default_font_size_.GetValue()); web_ui_->CallJavascriptFunction( "options.AdvancedOptions.SetFontSize", font_size); } +void AdvancedOptionsHandler::SetupPageZoomSelector() { + PrefService* pref_service = Profile::FromWebUI(web_ui_)->GetPrefs(); + double default_zoom_level = pref_service->GetDouble(prefs::kDefaultZoomLevel); + double default_zoom_factor = + WebKit::WebView::zoomLevelToZoomFactor(default_zoom_level); + + // Generate a vector of zoom factors from an array of known presets along with + // the default factor added if necessary. + std::vector<double> zoom_factors = + chrome_page_zoom::PresetZoomFactors(default_zoom_factor); + + // Iterate through the zoom factors and and build the contents of the + // selector that will be sent to the javascript handler. + // Each item in the list has the following parameters: + // 1. Title (string). + // 2. Value (double). + // 3. Is selected? (bool). + ListValue zoom_factors_value; + for (std::vector<double>::const_iterator i = zoom_factors.begin(); + i != zoom_factors.end(); ++i) { + ListValue* option = new ListValue(); + double factor = *i; + int percent = static_cast<int>(factor * 100 + 0.5); + option->Append(Value::CreateStringValue( + l10n_util::GetStringFUTF16Int(IDS_ZOOM_PERCENT, percent))); + option->Append(Value::CreateDoubleValue(factor)); + bool selected = content::ZoomValuesEqual(factor, default_zoom_factor); + option->Append(Value::CreateBooleanValue(selected)); + zoom_factors_value.Append(option); + } + + web_ui_->CallJavascriptFunction( + "options.AdvancedOptions.SetupPageZoomSelector", zoom_factors_value); +} + void AdvancedOptionsHandler::SetupAutoOpenFileTypesDisabledAttribute() { // Set the enabled state for the AutoOpenFileTypesResetToDefault button. // We enable the button if the user has any auto-open file types registered. diff --git a/chrome/browser/ui/webui/options/advanced_options_handler.h b/chrome/browser/ui/webui/options/advanced_options_handler.h index 0be1e22..6696c1d 100644 --- a/chrome/browser/ui/webui/options/advanced_options_handler.h +++ b/chrome/browser/ui/webui/options/advanced_options_handler.h @@ -48,23 +48,28 @@ class AdvancedOptionsHandler virtual void OnCloudPrintSetupClosed() OVERRIDE; private: - // Callback for the "selectDownloadLocation" message. This will prompt - // the user for a destination folder using platform-specific APIs. + // Callback for the "selectDownloadLocation" message. This will prompt the + // user for a destination folder using platform-specific APIs. void HandleSelectDownloadLocation(const ListValue* args); - // Callback for the "autoOpenFileTypesResetToDefault" message. This will + // Callback for the "autoOpenFileTypesResetToDefault" message. This will // remove all auto-open file-type settings. void HandleAutoOpenButton(const ListValue* args); - // Callback for the "metricsReportingCheckboxAction" message. This is called + // Callback for the "metricsReportingCheckboxAction" message. This is called // if the user toggles the metrics reporting checkbox. void HandleMetricsReportingCheckbox(const ListValue* args); - // Callback for the "defaultFontSizeAction" message. This is called if the - // user changes the default font size. |args| is an array that contains + // Callback for the "defaultFontSizeAction" message. This is called if the + // user changes the default font size. |args| is an array that contains // one item, the font size as a numeric value. void HandleDefaultFontSize(const ListValue* args); + // Callback for the "defaultZoomFactorAction" message. This is called if the + // user changes the default zoom factor. |args| is an array that contains + // one item, the zoom factor as a numeric value. + void HandleDefaultZoomFactor(const ListValue* args); + // Callback for the "Check for server certificate revocation" checkbox. This // is called if the user toggles the "Check for server certificate revocation" // checkbox. @@ -90,16 +95,16 @@ class AdvancedOptionsHandler void ShowManageSSLCertificates(const ListValue* args); #endif - // Callback for the Cloud Print manage button. This will open a new + // Callback for the Cloud Print manage button. This will open a new // tab pointed at the management URL. void ShowCloudPrintManagePage(const ListValue* args); #if !defined(OS_CHROMEOS) - // Callback for the Sign in to Cloud Print button. This will start + // Callback for the Sign in to Cloud Print button. This will start // the authentication process. void ShowCloudPrintSetupDialog(const ListValue* args); - // Callback for the Disable Cloud Print button. This will sign out + // Callback for the Disable Cloud Print button. This will sign out // of cloud print. void HandleDisableCloudPrintProxy(const ListValue* args); @@ -131,7 +136,11 @@ class AdvancedOptionsHandler // Setup the visibility for the metrics reporting setting. void SetupMetricsReportingSettingVisibility(); - void SetupFontSizeLabel(); + // Setup the font size selector control. + void SetupFontSizeSelector(); + + // Setup the page zoom selector control. + void SetupPageZoomSelector(); // Setup the enabled state of the reset button. void SetupAutoOpenFileTypesDisabledAttribute(); @@ -161,6 +170,7 @@ class AdvancedOptionsHandler StringPrefMember auto_open_files_; IntegerPrefMember default_font_size_; + DoublePrefMember default_zoom_level_; #if !defined(OS_CHROMEOS) scoped_ptr<PrefSetObserver> proxy_prefs_; diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index c248603..5e1c77a 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -367,6 +367,8 @@ 'browser/chrome_browser_main_extra_parts_views.h', 'browser/chrome_content_browser_client.cc', 'browser/chrome_content_browser_client.h', + 'browser/chrome_page_zoom.cc', + 'browser/chrome_page_zoom.h', 'browser/chrome_plugin_message_filter.cc', 'browser/chrome_plugin_message_filter.h', 'browser/chrome_plugin_service_filter.cc', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 945d1b3..7c0965d 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1269,6 +1269,7 @@ 'browser/browsing_data_remover_unittest.cc', 'browser/chrome_browser_application_mac_unittest.mm', 'browser/chrome_browser_main_unittest.cc', + 'browser/chrome_page_zoom_unittest.cc', 'browser/chromeos/cros/network_library.cc', 'browser/chromeos/cros/network_library.h', 'browser/chromeos/cros/network_library_unittest.cc', diff --git a/content/browser/host_zoom_map.cc b/content/browser/host_zoom_map.cc index b827433..c36aa4e 100644 --- a/content/browser/host_zoom_map.cc +++ b/content/browser/host_zoom_map.cc @@ -14,6 +14,7 @@ #include "content/public/browser/browser_thread.h" #include "content/public/browser/notification_service.h" #include "content/public/browser/notification_types.h" +#include "content/public/common/page_zoom.h" #include "googleurl/src/gurl.h" #include "net/base/net_util.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebView.h" @@ -56,7 +57,8 @@ void HostZoomMap::SetZoomLevel(std::string host, double level) { { base::AutoLock auto_lock(lock_); - if (level == default_zoom_level_) + + if (content::ZoomValuesEqual(level, default_zoom_level_)) host_zoom_levels_.erase(host); else host_zoom_levels_[host] = level; diff --git a/content/browser/host_zoom_map_unittest.cc b/content/browser/host_zoom_map_unittest.cc new file mode 100644 index 0000000..d151362 --- /dev/null +++ b/content/browser/host_zoom_map_unittest.cc @@ -0,0 +1,32 @@ +// Copyright (c) 2011 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. + +#include "content/browser/host_zoom_map.h" + +#include "base/memory/ref_counted.h" +#include "base/message_loop.h" +#include "content/public/browser/browser_thread.h" +#include "content/test/test_browser_thread.h" +#include "testing/gtest/include/gtest/gtest.h" + +class HostZoomMapTest : public testing::Test { + public: + HostZoomMapTest() : ui_thread_(content::BrowserThread::UI, &message_loop_) { + } + + protected: + MessageLoop message_loop_; + content::TestBrowserThread ui_thread_; +}; + +TEST_F(HostZoomMapTest, GetSetZoomLevel) { + scoped_refptr<HostZoomMap> host_zoom_map = new HostZoomMap; + + double zoomed = 2.5; + host_zoom_map->SetZoomLevel("zoomed.com", zoomed); + + EXPECT_DOUBLE_EQ(host_zoom_map->GetZoomLevel("normal.com"), 0); + EXPECT_DOUBLE_EQ(host_zoom_map->GetZoomLevel("zoomed.com"), zoomed); +} + diff --git a/content/browser/tab_contents/tab_contents.cc b/content/browser/tab_contents/tab_contents.cc index eb1f934..5fe45a9 100644 --- a/content/browser/tab_contents/tab_contents.cc +++ b/content/browser/tab_contents/tab_contents.cc @@ -197,9 +197,9 @@ TabContents::TabContents(content::BrowserContext* browser_context, opener_web_ui_type_(WebUI::kNoWebUI), closed_by_user_gesture_(false), minimum_zoom_percent_( - static_cast<int>(WebKit::WebView::minTextSizeMultiplier * 100)), + static_cast<int>(content::kMinimumZoomFactor * 100)), maximum_zoom_percent_( - static_cast<int>(WebKit::WebView::maxTextSizeMultiplier * 100)), + static_cast<int>(content::kMaximumZoomFactor * 100)), temporary_zoom_settings_(false), content_restrictions_(0), view_type_(content::VIEW_TYPE_TAB_CONTENTS) { @@ -862,8 +862,10 @@ double TabContents::GetZoomLevel() const { int TabContents::GetZoomPercent(bool* enable_increment, bool* enable_decrement) { *enable_decrement = *enable_increment = false; + // Calculate the zoom percent from the factor. Round up to the nearest whole + // number. int percent = static_cast<int>( - WebKit::WebView::zoomLevelToZoomFactor(GetZoomLevel()) * 100); + WebKit::WebView::zoomLevelToZoomFactor(GetZoomLevel()) * 100 + 0.5); *enable_decrement = percent > minimum_zoom_percent_; *enable_increment = percent < maximum_zoom_percent_; return percent; diff --git a/content/browser/webui/web_ui.cc b/content/browser/webui/web_ui.cc index 5febef8..38e9839 100644 --- a/content/browser/webui/web_ui.cc +++ b/content/browser/webui/web_ui.cc @@ -228,6 +228,15 @@ bool WebUIMessageHandler::ExtractIntegerValue(const ListValue* value, return false; } +bool WebUIMessageHandler::ExtractDoubleValue(const ListValue* value, + double* out_value) { + std::string string_value; + if (value->GetString(0, &string_value)) + return base::StringToDouble(string_value, out_value); + NOTREACHED(); + return false; +} + string16 WebUIMessageHandler::ExtractStringValue(const ListValue* value) { string16 string16_value; if (value->GetString(0, &string16_value)) diff --git a/content/browser/webui/web_ui.h b/content/browser/webui/web_ui.h index 998513f..5cba59f 100644 --- a/content/browser/webui/web_ui.h +++ b/content/browser/webui/web_ui.h @@ -219,6 +219,9 @@ class CONTENT_EXPORT WebUIMessageHandler { // Extract an integer value from a list Value. bool ExtractIntegerValue(const base::ListValue* value, int* out_int); + // Extract a floating point (double) value from a list Value. + bool ExtractDoubleValue(const base::ListValue* value, double* out_value); + // Extract a string value from a list Value. string16 ExtractStringValue(const base::ListValue* value); diff --git a/content/browser/webui/web_ui_unittest.cc b/content/browser/webui/web_ui_unittest.cc new file mode 100644 index 0000000..93e597f --- /dev/null +++ b/content/browser/webui/web_ui_unittest.cc @@ -0,0 +1,74 @@ +// Copyright (c) 2011 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. + +#include "content/browser/webui/web_ui.h" + +#include "base/string16.h" +#include "base/values.h" +#include "testing/gtest/include/gtest/gtest.h" + +class TestWebUIMessageHandler : public WebUIMessageHandler { + public: + TestWebUIMessageHandler() {} + virtual ~TestWebUIMessageHandler() {} + + protected: + virtual void RegisterMessages() {} + + private: + DISALLOW_COPY_AND_ASSIGN(TestWebUIMessageHandler); +}; + +TEST(WebUIMessageHandlerTest, ExtractIntegerValue) { + TestWebUIMessageHandler handler; + + ListValue list; + int value, zero_value = 0, neg_value = -1234, pos_value = 1234; + + list.Append(Value::CreateIntegerValue(zero_value)); + EXPECT_TRUE(handler.ExtractIntegerValue(&list, &value)); + EXPECT_EQ(value, zero_value); + list.Clear(); + + list.Append(Value::CreateIntegerValue(neg_value)); + EXPECT_TRUE(handler.ExtractIntegerValue(&list, &value)); + EXPECT_EQ(value, neg_value); + list.Clear(); + + list.Append(Value::CreateIntegerValue(pos_value)); + EXPECT_TRUE(handler.ExtractIntegerValue(&list, &value)); + EXPECT_EQ(value, pos_value); +} + +TEST(WebUIMessageHandlerTest, ExtractDoubleValue) { + TestWebUIMessageHandler handler; + + ListValue list; + double value, zero_value = 0.0, neg_value = -1234.5, pos_value = 1234.5; + + list.Append(Value::CreateDoubleValue(zero_value)); + EXPECT_TRUE(handler.ExtractDoubleValue(&list, &value)); + EXPECT_EQ(value, zero_value); + list.Clear(); + + list.Append(Value::CreateDoubleValue(neg_value)); + EXPECT_TRUE(handler.ExtractDoubleValue(&list, &value)); + EXPECT_EQ(value, neg_value); + list.Clear(); + + list.Append(Value::CreateDoubleValue(pos_value)); + EXPECT_TRUE(handler.ExtractDoubleValue(&list, &value)); + EXPECT_EQ(value, pos_value); +} + +TEST(WebUIMessageHandlerTest, ExtractStringValue) { + TestWebUIMessageHandler handler; + + ListValue list; + string16 in_string = "The facts, though interesting, are irrelevant." + list.Append(Value::CreateStringValue(string)); + string16 out_string = handler.ExtractStringValue(&list); + EXPECT_STREQ(in_string.c_str(), out_string.c_str()); +} + diff --git a/content/common/page_zoom.cc b/content/common/page_zoom.cc new file mode 100644 index 0000000..d9b6b51 --- /dev/null +++ b/content/common/page_zoom.cc @@ -0,0 +1,25 @@ +// Copyright (c) 2011 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. + +#include <cmath> + +#include "content/public/common/page_zoom.h" + +namespace content { + +const double kMinimumZoomFactor = 0.25; +const double kMaximumZoomFactor = 5.0; + +bool ZoomValuesEqual(double value_a, double value_b) { + // Epsilon value for comparing two floating-point zoom values. We don't use + // std::numeric_limits<> because it is too precise for zoom values. Zoom + // values lose precision due to factor/level conversions. A value of 0.001 + // is precise enough for zoom value comparisons. + const double epsilon = 0.001; + + return (std::fabs(value_a - value_b) <= epsilon); +} + +} // namespace content + diff --git a/content/common/page_zoom_unittest.cc b/content/common/page_zoom_unittest.cc new file mode 100644 index 0000000..67dd5d5 --- /dev/null +++ b/content/common/page_zoom_unittest.cc @@ -0,0 +1,19 @@ +// Copyright (c) 2011 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. + +#include "content/public/common/page_zoom.h" + +#include "testing/gtest/include/gtest/gtest.h" + +TEST(PageZoomTest, ZoomValuesEqual) { + // Test two identical values. + EXPECT_TRUE(content::ZoomValuesEqual(1.5, 1.5)); + + // Test two values that are close enough to be considered equal. + EXPECT_TRUE(content::ZoomValuesEqual(1.5, 1.49999999)); + + // Test two values that are close, but should not be considered equal. + EXPECT_FALSE(content::ZoomValuesEqual(1.5, 1.4)); +} + diff --git a/content/content_common.gypi b/content/content_common.gypi index e42f568..f1f0328 100644 --- a/content/content_common.gypi +++ b/content/content_common.gypi @@ -192,6 +192,7 @@ 'common/npobject_util.h', 'common/p2p_messages.h', 'common/p2p_sockets.h', + 'common/page_zoom.cc', 'common/pepper_file_messages.cc', 'common/pepper_file_messages.h', 'common/pepper_messages.h', diff --git a/content/content_tests.gypi b/content/content_tests.gypi index 520654a..e95c636 100644 --- a/content/content_tests.gypi +++ b/content/content_tests.gypi @@ -187,6 +187,7 @@ 'browser/geolocation/win7_location_api_unittest_win.cc', 'browser/geolocation/win7_location_provider_unittest_win.cc', 'browser/gpu/gpu_blacklist_unittest.cc', + 'browser/host_zoom_map_unittest.cc', 'browser/in_process_webkit/dom_storage_unittest.cc', 'browser/in_process_webkit/indexed_db_quota_client_unittest.cc', 'browser/in_process_webkit/webkit_context_unittest.cc', @@ -224,6 +225,7 @@ 'common/gpu/gpu_info_unittest.cc', 'common/hi_res_timer_manager_unittest.cc', 'common/net/url_fetcher_impl_unittest.cc', + 'common/page_zoom_unittest.cc', 'common/process_watcher_unittest.cc', 'common/property_bag_unittest.cc', 'common/resource_dispatcher_unittest.cc', diff --git a/content/public/common/page_zoom.h b/content/public/common/page_zoom.h index 956169f..3abe75a 100644 --- a/content/public/common/page_zoom.h +++ b/content/public/common/page_zoom.h @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -6,7 +6,7 @@ #define CONTENT_PUBLIC_COMMON_PAGE_ZOOM_H_ #pragma once -#include "base/basictypes.h" +#include "content/common/content_export.h" namespace content { @@ -18,6 +18,18 @@ enum PageZoom { PAGE_ZOOM_IN = 1, }; +// The minimum zoom factor permitted for a page. This is an alternative to +// WebView::minTextSizeMultiplier. +CONTENT_EXPORT extern const double kMinimumZoomFactor; + +// The maximum zoom factor permitted for a page. This is an alternative to +// WebView::maxTextSizeMultiplier. +CONTENT_EXPORT extern const double kMaximumZoomFactor; + +// Test if two zoom values (either zoom factors or zoom levels) should be +// considered equal. +CONTENT_EXPORT bool ZoomValuesEqual(double value_a, double value_b); + } // namespace content #endif // CONTENT_PUBLIC_COMMON_PAGE_ZOOM_H_ diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc index 7eeaa8d..244bf8b 100644 --- a/content/renderer/render_view_impl.cc +++ b/content/renderer/render_view_impl.cc @@ -1111,8 +1111,8 @@ void RenderViewImpl::UpdateURL(WebFrame* frame) { // will also call us back which will cause us to send a message to // update TabContents. webview()->zoomLimitsChanged( - WebView::zoomFactorToZoomLevel(WebView::minTextSizeMultiplier), - WebView::zoomFactorToZoomLevel(WebView::maxTextSizeMultiplier)); + WebView::zoomFactorToZoomLevel(content::kMinimumZoomFactor), + WebView::zoomFactorToZoomLevel(content::kMaximumZoomFactor)); // Update contents MIME type for main frame. params.contents_mime_type = ds->response().mimeType().utf8(); |