Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(843)

Issue 33753004: Add link to the Advanced Font Settings extension in Font Settings (Closed)

Created:
7 years, 2 months ago by falken
Modified:
7 years, 1 month ago
Reviewers:
Matt Perry, Dan Beam
CC:
chromium-reviews, dbeam+watch-options_chromium.org, arv+watch_chromium.org
Visibility:
Public.

Description

Add link to the Advanced Font Settings extension in Font Settings When the extension is installed and enabled, the link points to the extension's options page. Otherwise, the link points to the extension's page on the Chrome Web Store. This implements the design described in crbug.com/272216 BUG=138993, 272216 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=231017

Patch Set 1 #

Total comments: 1

Patch Set 2 : rm uneeded include #

Total comments: 23

Patch Set 3 : review comments #

Total comments: 4

Patch Set 4 : review comments #

Patch Set 5 : review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+150 lines, -0 lines) Patch
M chrome/app/generated_resources.grd View 1 chunk +8 lines, -0 lines 0 comments Download
M chrome/browser/resources/options/font_settings.css View 1 chunk +13 lines, -0 lines 0 comments Download
M chrome/browser/resources/options/font_settings.html View 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/resources/options/font_settings.js View 1 2 2 chunks +13 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/options/font_settings_browsertest.js View 1 chunk +24 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/options/font_settings_handler.h View 1 2 4 chunks +23 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/options/font_settings_handler.cc View 1 2 3 7 chunks +64 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
falken
dbeam: Can you review as WebUI owner? mpcomplete: Can you review the extensions bits? Thanks, ...
7 years, 2 months ago (2013-10-23 05:50:23 UTC) #1
Dan Beam
https://codereview.chromium.org/33753004/diff/60001/chrome/browser/resources/options/font_settings.css File chrome/browser/resources/options/font_settings.css (right): https://codereview.chromium.org/33753004/diff/60001/chrome/browser/resources/options/font_settings.css#newcode15 chrome/browser/resources/options/font_settings.css:15: display: -webkit-box; any reason you're not using the new ...
7 years, 2 months ago (2013-10-23 18:03:59 UTC) #2
falken
Thanks for the review! PTAL https://codereview.chromium.org/33753004/diff/60001/chrome/browser/resources/options/font_settings.css File chrome/browser/resources/options/font_settings.css (right): https://codereview.chromium.org/33753004/diff/60001/chrome/browser/resources/options/font_settings.css#newcode15 chrome/browser/resources/options/font_settings.css:15: display: -webkit-box; On 2013/10/23 ...
7 years, 2 months ago (2013-10-24 12:09:41 UTC) #3
Dan Beam
https://chromiumcodereview.appspot.com/33753004/diff/170001/chrome/browser/ui/webui/options/font_settings_handler.cc File chrome/browser/ui/webui/options/font_settings_handler.cc (right): https://chromiumcodereview.appspot.com/33753004/diff/170001/chrome/browser/ui/webui/options/font_settings_handler.cc#newcode290 chrome/browser/ui/webui/options/font_settings_handler.cc:290: extensions::ExtensionSystem::Get(profile)->extension_service(); nit: if (!service->IsExtensionEnabled(kAdvancedFontSettingsExtensionId)) return NULL; return service->GetInstalledExtension(kAdvancedFontSettingsExtensionId); https://chromiumcodereview.appspot.com/33753004/diff/170001/chrome/browser/ui/webui/options/font_settings_handler.h ...
7 years, 1 month ago (2013-10-25 02:29:59 UTC) #4
Dan Beam
lgtm
7 years, 1 month ago (2013-10-25 02:30:04 UTC) #5
falken
https://codereview.chromium.org/33753004/diff/170001/chrome/browser/ui/webui/options/font_settings_handler.cc File chrome/browser/ui/webui/options/font_settings_handler.cc (right): https://codereview.chromium.org/33753004/diff/170001/chrome/browser/ui/webui/options/font_settings_handler.cc#newcode290 chrome/browser/ui/webui/options/font_settings_handler.cc:290: extensions::ExtensionSystem::Get(profile)->extension_service(); On 2013/10/25 02:29:59, Dan Beam wrote: > nit: ...
7 years, 1 month ago (2013-10-25 12:01:28 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/falken@chromium.org/33753004/360001
7 years, 1 month ago (2013-10-25 12:03:21 UTC) #7
commit-bot: I haz the power
7 years, 1 month ago (2013-10-25 14:50:21 UTC) #8
Message was sent while issue was closed.
Change committed as 231017

Powered by Google App Engine
This is Rietveld 408576698