Skip to content

Commit f0f4366

Browse files
committed
[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} (cherry picked from commit b7cd6a5) Review-Url: https://codereview.chromium.org/2830883002 . Cr-Commit-Position: refs/branch-heads/3071@{#64} Cr-Branched-From: a106f0a-refs/heads/master@{#464641}
1 parent 5d12d72 commit f0f4366

File tree

4 files changed

+32
-41
lines changed

4 files changed

+32
-41
lines changed

chrome/browser/resources/settings/appearance_page/appearance_fonts_page.html

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,13 +111,11 @@ <h2>$i18n{fixedWidthFont}</h2>
111111
</div>
112112
<div class="list-item"
113113
style="
114-
font-size:[[prefs.webkit.webprefs.default_font_size.value]]px;
114+
font-size:
115+
[[prefs.webkit.webprefs.default_fixed_font_size.value]]px;
115116
font-family:
116117
'[[prefs.webkit.webprefs.fonts.fixed.Zyyy.value]]';">
117-
<span>
118-
[[prefs.webkit.webprefs.default_font_size.value]]:
119-
$i18n{quickBrownFox}
120-
</span>
118+
$i18n{quickBrownFox}
121119
</div>
122120
</div>
123121
<template is="dom-if" if="[[!isGuest_]]">

chrome/browser/resources/settings/appearance_page/appearance_fonts_page.js

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,6 @@
55
(function() {
66
'use strict';
77

8-
/**
9-
* This is the absolute difference maintained between standard and
10-
* fixed-width font sizes. http://crbug.com/91922.
11-
* @const @private {number}
12-
*/
13-
var SIZE_DIFFERENCE_FIXED_STANDARD_ = 3;
14-
158
/** @const @private {!Array<number>} */
169
var FONT_SIZE_RANGE_ = [
1710
9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 20, 22, 24, 26, 28, 30, 32, 34, 36,
@@ -26,11 +19,6 @@
2619
/**
2720
* 'settings-appearance-fonts-page' is the settings page containing appearance
2821
* settings.
29-
*
30-
* Example:
31-
*
32-
* <settings-appearance-fonts-page prefs="{{prefs}}">
33-
* </settings-appearance-fonts-page>
3422
*/
3523
Polymer({
3624
is: 'settings-appearance-fonts-page',
@@ -85,10 +73,6 @@
8573
},
8674
},
8775

88-
observers: [
89-
'fontSizeChanged_(prefs.webkit.webprefs.default_font_size.value)',
90-
],
91-
9276
/** @private {?settings.FontsBrowserProxy} */
9377
browserProxy_: null,
9478

@@ -143,18 +127,6 @@
143127
this.advancedExtensionUrl_ = response.extensionUrl;
144128
},
145129

146-
/**
147-
* @param {number} value The changed font size slider value.
148-
* @private
149-
*/
150-
fontSizeChanged_: function(value) {
151-
// TODO(michaelpg): Whitelist this pref in prefs_utils.cc so it is
152-
// included in the <settings-prefs> getAllPrefs call, otherwise this path
153-
// is invalid and nothing happens. See crbug.com/612535.
154-
this.set('prefs.webkit.webprefs.default_fixed_font_size.value',
155-
value - SIZE_DIFFERENCE_FIXED_STANDARD_);
156-
},
157-
158130
/**
159131
* Get the minimum font size, accounting for unset prefs.
160132
* @return {?}

chrome/browser/resources/settings/appearance_page/appearance_page.html

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,7 @@
173173
<button class="subpage-arrow" is="paper-icon-button-light"
174174
aria-label="$i18n{customizeFonts}"></button>
175175
</div>
176-
<div class="settings-box"
177-
hidden="[[!pageVisibility.pageZoom]]">
176+
<div class="settings-box" hidden="[[!pageVisibility.pageZoom]]">
178177
<div id="pageZoom" class="start">$i18n{pageZoom}</div>
179178
<div class="md-select-wrapper">
180179
<select id="zoomLevel" class="md-select" aria-labelledy="pageZoom"

chrome/browser/resources/settings/appearance_page/appearance_page.js

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,15 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
6+
/**
7+
* This is the absolute difference maintained between standard and
8+
* fixed-width font sizes. http://crbug.com/91922.
9+
* @const @private {number}
10+
*/
11+
var SIZE_DIFFERENCE_FIXED_STANDARD_ = 3;
12+
13+
514
/**
615
* 'settings-appearance-page' is the settings page containing appearance
716
* settings.
@@ -110,12 +119,13 @@ Polymer({
110119
themeUrl_: '',
111120

112121
observers: [
122+
'defaultFontSizeChanged_(prefs.webkit.webprefs.default_font_size.value)',
113123
'themeChanged_(prefs.extensions.theme.id.value, useSystemTheme_)',
114124

115-
// <if expr="is_linux and not chromeos">
125+
// <if expr="is_linux and not chromeos">
116126
// NOTE: this pref only exists on Linux.
117127
'useSystemThemePrefChanged_(prefs.extensions.theme.use_system.value)',
118-
// </if>
128+
// </if>
119129
],
120130

121131
created: function() {
@@ -165,6 +175,18 @@ Polymer({
165175
this.fire('refresh-pref', 'homepage');
166176
},
167177

178+
/**
179+
* @param {number} value The changed font size slider value.
180+
* @private
181+
*/
182+
defaultFontSizeChanged_: function(value) {
183+
// This pref is handled separately in some extensions, but here it is tied
184+
// to default_font_size (to simplify the UI).
185+
this.set(
186+
'prefs.webkit.webprefs.default_fixed_font_size.value',
187+
value - SIZE_DIFFERENCE_FIXED_STANDARD_);
188+
},
189+
168190
/** @private */
169191
onThemesTap_: function() {
170192
window.open(this.themeUrl_ || loadTimeData.getString('themesGalleryUrl'));
@@ -250,12 +272,12 @@ Polymer({
250272
}
251273

252274
var i18nId;
253-
// <if expr="is_linux and not chromeos">
275+
// <if expr="is_linux and not chromeos">
254276
i18nId = useSystemTheme ? 'systemTheme' : 'classicTheme';
255-
// </if>
256-
// <if expr="not is_linux or chromeos">
277+
// </if>
278+
// <if expr="not is_linux or chromeos">
257279
i18nId = 'chooseFromWebStore';
258-
// </if>
280+
// </if>
259281
this.themeSublabel_ = this.i18n(i18nId);
260282
this.themeUrl_ = '';
261283
},

0 commit comments

Comments
 (0)