[MD settings] observe changes to default font size in Appearance

This CL updates the fixed font size from the Appearance section reliably. (It was previously working depending on which settings page had been visited). Also, updates the UI to remove the font size on the example of the fixed font size. BUG=671562 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2817243003 Cr-Commit-Position: refs/heads/master@{#465054}
nwjs · Apr 17, 2017 · b7cd6a5 · b7cd6a5
1 parent fa3c7ef
commit b7cd6a5
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 41 deletions.
diff --git a/chrome/browser/resources/settings/appearance_page/appearance_fonts_page.html b/chrome/browser/resources/settings/appearance_page/appearance_fonts_page.html
@@ -111,13 +111,11 @@ <h2>$i18n{fixedWidthFont}</h2>
       </div>
       <div class="list-item"
           style="
-              font-size:[[prefs.webkit.webprefs.default_font_size.value]]px;
+              font-size:
+                  [[prefs.webkit.webprefs.default_fixed_font_size.value]]px;
               font-family:
                   '[[prefs.webkit.webprefs.fonts.fixed.Zyyy.value]]';">
-        <span>
-          [[prefs.webkit.webprefs.default_font_size.value]]:
-          $i18n{quickBrownFox}
-        </span>
+        $i18n{quickBrownFox}
       </div>
     </div>
     <template is="dom-if" if="[[!isGuest_]]">

diff --git a/chrome/browser/resources/settings/appearance_page/appearance_fonts_page.js b/chrome/browser/resources/settings/appearance_page/appearance_fonts_page.js
@@ -5,13 +5,6 @@
 (function() {
   'use strict';
 
-  /**
-   * This is the absolute difference maintained between standard and
-   * fixed-width font sizes. http://crbug.com/91922.
-   * @const @private {number}
-   */
-  var SIZE_DIFFERENCE_FIXED_STANDARD_ = 3;
-
   /** @const @private {!Array<number>} */
   var FONT_SIZE_RANGE_ = [
     9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 20, 22, 24, 26, 28, 30, 32, 34, 36,
@@ -26,11 +19,6 @@
   /**
    * 'settings-appearance-fonts-page' is the settings page containing appearance
    * settings.
-   *
-   * Example:
-   *
-   *   <settings-appearance-fonts-page prefs="{{prefs}}">
-   *   </settings-appearance-fonts-page>
    */
   Polymer({
     is: 'settings-appearance-fonts-page',
@@ -88,10 +76,6 @@
     /** @private {?string} */
     advancedExtensionUrl_: null,
 
-    observers: [
-      'fontSizeChanged_(prefs.webkit.webprefs.default_font_size.value)',
-    ],
-
     /** @override */
     created: function() {
       this.browserProxy_ = settings.FontsBrowserProxyImpl.getInstance();
@@ -143,18 +127,6 @@
       this.advancedExtensionUrl_ = response.extensionUrl;
     },
 
-    /**
-     * @param {number} value The changed font size slider value.
-     * @private
-     */
-    fontSizeChanged_: function(value) {
-      // TODO(michaelpg): Whitelist this pref in prefs_utils.cc so it is
-      // included in the <settings-prefs> getAllPrefs call, otherwise this path
-      // is invalid and nothing happens. See crbug.com/612535.
-      this.set('prefs.webkit.webprefs.default_fixed_font_size.value',
-          value - SIZE_DIFFERENCE_FIXED_STANDARD_);
-    },
-
     /**
      * Get the minimum font size, accounting for unset prefs.
      * @return {?}

diff --git a/chrome/browser/resources/settings/appearance_page/appearance_page.html b/chrome/browser/resources/settings/appearance_page/appearance_page.html
@@ -173,8 +173,7 @@
           <button class="subpage-arrow" is="paper-icon-button-light"
               aria-label="$i18n{customizeFonts}"></button>
         </div>
-        <div class="settings-box"
-            hidden="[[!pageVisibility.pageZoom]]">
+        <div class="settings-box" hidden="[[!pageVisibility.pageZoom]]">
           <div id="pageZoom" class="start">$i18n{pageZoom}</div>
           <div class="md-select-wrapper">
             <select id="zoomLevel" class="md-select" aria-labelledy="pageZoom"

diff --git a/chrome/browser/resources/settings/appearance_page/appearance_page.js b/chrome/browser/resources/settings/appearance_page/appearance_page.js
@@ -2,6 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+
+/**
+ * This is the absolute difference maintained between standard and
+ * fixed-width font sizes. http://crbug.com/91922.
+ * @const @private {number}
+ */
+var SIZE_DIFFERENCE_FIXED_STANDARD_ = 3;
+
+
 /**
  * 'settings-appearance-page' is the settings page containing appearance
  * settings.
@@ -110,12 +119,13 @@ Polymer({
   themeUrl_: '',
 
   observers: [
+    'defaultFontSizeChanged_(prefs.webkit.webprefs.default_font_size.value)',
     'themeChanged_(prefs.extensions.theme.id.value, useSystemTheme_)',
 
-// <if expr="is_linux and not chromeos">
+    // <if expr="is_linux and not chromeos">
     // NOTE: this pref only exists on Linux.
     'useSystemThemePrefChanged_(prefs.extensions.theme.use_system.value)',
-// </if>
+    // </if>
   ],
 
   created: function() {
@@ -165,6 +175,18 @@ Polymer({
     this.fire('refresh-pref', 'homepage');
   },
 
+  /**
+   * @param {number} value The changed font size slider value.
+   * @private
+   */
+  defaultFontSizeChanged_: function(value) {
+    // This pref is handled separately in some extensions, but here it is tied
+    // to default_font_size (to simplify the UI).
+    this.set(
+        'prefs.webkit.webprefs.default_fixed_font_size.value',
+        value - SIZE_DIFFERENCE_FIXED_STANDARD_);
+  },
+
   /** @private */
   onThemesTap_: function() {
     window.open(this.themeUrl_ || loadTimeData.getString('themesGalleryUrl'));
@@ -250,12 +272,12 @@ Polymer({
     }
 
     var i18nId;
-// <if expr="is_linux and not chromeos">
+    // <if expr="is_linux and not chromeos">
     i18nId = useSystemTheme ? 'systemTheme' : 'classicTheme';
-// </if>
-// <if expr="not is_linux or chromeos">
+    // </if>
+    // <if expr="not is_linux or chromeos">
     i18nId = 'chooseFromWebStore';
-// </if>
+    // </if>
     this.themeSublabel_ = this.i18n(i18nId);
     this.themeUrl_ = '';
   },