Issue 9853013: Expose more font size prefs to the Font Settings Extension API.

Issue 9853013: Expose more font size prefs to the Font Settings Extension API. (Closed)

Created:
8 years, 9 months ago by falken

Modified:
8 years, 9 months ago

Reviewers:
Matt Perry

CC:
chromium-reviews, Aaron Boodman, mihaip+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Expose more font size prefs to the Font Settings Extension API. This adds functions for default fixed font size and minimum font size. BUG=114148 TEST=browser_tests --gtest_filter=ExtensionApiTest.FontSettings Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=129125

Patch Set 1 #

Patch Set 2 : retry #

Total comments: 4

Patch Set 3 : review comments #

Patch Set 4 : "true" to true #

Created: 8 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+751 lines, -13 lines)			Patch
M	chrome/browser/extensions/extension_font_settings_api.h	View	1 2	1 chunk	+62 lines, -4 lines	0 comments	Download
M	chrome/browser/extensions/extension_font_settings_api.cc	View	1 2	3 chunks	+28 lines, -4 lines	0 comments	Download
M	chrome/browser/extensions/extension_function_registry.cc	View	1 2	1 chunk	+4 lines, -0 lines	0 comments	Download
M	chrome/common/extensions/api/experimental.fontSettings.json	View	1 2 3	4 chunks	+111 lines, -4 lines	0 comments	Download
M	chrome/common/extensions/docs/experimental.fontSettings.html	View	1 2	5 chunks	+512 lines, -1 line	0 comments	Download
M	chrome/common/extensions/docs/samples.json	View		1 chunk	+4 lines, -0 lines	0 comments	Download
M	chrome/test/data/extensions/api_test/font_settings/test.js	View	1 2	1 chunk	+30 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Matt Perry

lgtm https://chromiumcodereview.appspot.com/9853013/diff/2002/chrome/browser/extensions/extension_font_settings_api.h File chrome/browser/extensions/extension_font_settings_api.h (right): https://chromiumcodereview.appspot.com/9853013/diff/2002/chrome/browser/extensions/extension_font_settings_api.h#newcode39 chrome/browser/extensions/extension_font_settings_api.h:39: virtual const char* pref_name() = 0; underscore_style() is ...

8 years, 9 months ago (2012-03-26 18:53:05 UTC) #2

falken

8 years, 9 months ago (2012-03-27 02:26:01 UTC) #3

Thanks!

http://codereview.chromium.org/9853013/diff/2002/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_font_settings_api.h (right):

http://codereview.chromium.org/9853013/diff/2002/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_font_settings_api.h:39: virtual const char*
pref_name() = 0;
On 2012/03/26 18:53:05, Matt Perry wrote:
> underscore_style() is reserved for inline methods. Use GetPrefName()

Done.

http://codereview.chromium.org/9853013/diff/2002/chrome/test/data/extensions/...
File chrome/test/data/extensions/api_test/font_settings/test.js (right):

http://codereview.chromium.org/9853013/diff/2002/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/font_settings/test.js:94:
fs.getMinimumFontSize({}, expect({pixelSize: expected}, message));
On 2012/03/26 18:53:05, Matt Perry wrote:
> how about putting all the setter tests before the getters, so you know that
each
> one is controlling an independent pref.

Done.

Expand Messages | Collapse Messages