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

Issue 549303004: Allow default font size changing on Linux. (Closed)

Created:
6 years, 3 months ago by joleksy
Modified:
5 years, 3 months ago
CC:
chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, jam, nasko+codewatch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Allow default font size changing on Linux. Currently base rendering font size on Linux is hardcoded to 16.0 with no possibility of changing it. This patch add such possibility in RendererPreferences. Webkit part: https://codereview.chromium.org/570243002/ BUG= Committed: https://crrev.com/f0bc1a926d8f0326ef38b8374e42bc8e244dd2ba Cr-Commit-Position: refs/heads/master@{#344746}

Patch Set 1 #

Total comments: 1

Patch Set 2 : CHECK font size. #

Patch Set 3 : Use PlatformFont::CreateDefault() for default font size. #

Patch Set 4 : Rebase #

Patch Set 5 : Remove CHECKs #

Total comments: 1

Patch Set 6 : Add braces. #

Patch Set 7 : Remove failing test. #

Patch Set 8 : Disabled test instead of removing. #

Patch Set 9 : Rebase #

Patch Set 10 : Win compile fix. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -1 line) Patch
M chrome/browser/renderer_preferences_util.cc View 1 2 3 4 5 6 7 8 9 2 chunks +7 lines, -0 lines 0 comments Download
M chrome/browser/resources/chromeos/chromevox/common/editable_text_area_shadow_test.unitjs View 1 2 3 4 5 6 7 1 chunk +3 lines, -1 line 0 comments Download
M content/common/view_messages.h View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M content/public/common/renderer_preferences.h View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
M content/public/common/renderer_preferences.cc View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M content/renderer/render_view_linux.cc View 1 2 3 4 5 6 7 8 2 chunks +6 lines, -0 lines 0 comments Download

Messages

Total messages: 67 (22 generated)
joleksy
@reviewer: does it look ok to you?
6 years, 3 months ago (2014-09-09 08:35:07 UTC) #2
jln (very slow on Chromium)
I am not a good reviewer for this. You would probably want to add a ...
6 years, 3 months ago (2014-09-11 20:40:12 UTC) #3
joleksy
On 2014/09/11 20:40:12, jln wrote: > I am not a good reviewer for this. You ...
6 years, 3 months ago (2014-09-12 06:26:50 UTC) #5
jamesr
I'm not a content/ reviewer either.
6 years, 3 months ago (2014-09-13 00:55:46 UTC) #7
joleksy
On 2014/09/13 00:55:46, jamesr wrote: > I'm not a content/ reviewer either. I am not ...
6 years, 3 months ago (2014-09-15 13:39:30 UTC) #9
jochen (gone - plz use gerrit)
lgtm still needs an IPC review
6 years, 3 months ago (2014-09-15 14:10:29 UTC) #10
jln (very slow on Chromium)
https://chromiumcodereview.appspot.com/549303004/diff/1/content/renderer/render_view_linux.cc File content/renderer/render_view_linux.cc (right): https://chromiumcodereview.appspot.com/549303004/diff/1/content/renderer/render_view_linux.cc#newcode94 content/renderer/render_view_linux.cc:94: WebFontRendering::setDefaultFontSize(prefs.default_font_size); Could you add some basic sanity-checking on the ...
6 years, 3 months ago (2014-09-15 19:25:10 UTC) #11
jochen (gone - plz use gerrit)
wait, can you explain how this interacts with WebSettings::setDefaultFontSize()? can we avoid the duplication somehow? ...
6 years, 3 months ago (2014-09-16 14:51:24 UTC) #12
joleksy
On 2014/09/16 14:51:24, jochen wrote: > wait, can you explain how this interacts with WebSettings::setDefaultFontSize()? ...
6 years, 3 months ago (2014-09-17 07:17:20 UTC) #13
jochen (gone - plz use gerrit)
On 2014/09/17 at 07:17:20, joleksy wrote: > On 2014/09/16 14:51:24, jochen wrote: > > wait, ...
6 years, 3 months ago (2014-09-17 14:53:52 UTC) #14
joleksy
On 2014/09/17 14:53:52, jochen wrote: > On 2014/09/17 at 07:17:20, joleksy wrote: > > On ...
6 years, 3 months ago (2014-09-18 06:22:16 UTC) #15
jochen (gone - plz use gerrit)
+erg ah, makes sense. Elliot, is there a way to figure out the default font ...
6 years, 3 months ago (2014-09-18 18:31:02 UTC) #18
Elliot Glaysher
On 2014/09/18 18:31:02, jochen wrote: > +erg > > ah, makes sense. > > Elliot, ...
6 years, 3 months ago (2014-09-18 19:05:42 UTC) #19
joleksy
On 2014/09/18 19:05:42, Elliot Glaysher wrote: > On 2014/09/18 18:31:02, jochen wrote: > > +erg ...
6 years, 2 months ago (2014-10-07 11:51:41 UTC) #20
jochen (gone - plz use gerrit)
I didn't realize you had uploaded a new patchset version. lgtm
6 years, 2 months ago (2014-10-07 12:13:47 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/549303004/40001
6 years, 2 months ago (2014-10-07 12:18:38 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: mac_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/65309) win_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/70177) android_aosp ...
6 years, 2 months ago (2014-10-07 12:23:38 UTC) #25
joleksy
Needs to be rebased, but anyway cannot be integrated before Webkit part (https://codereview.chromium.org/570243002/).
6 years, 2 months ago (2014-10-07 12:58:43 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/549303004/60001
6 years, 2 months ago (2014-10-08 06:23:56 UTC) #28
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/16220)
6 years, 2 months ago (2014-10-08 06:30:16 UTC) #30
joleksy
On 2014/10/08 06:30:16, I haz the power (commit-bot) wrote: > Try jobs failed on following ...
6 years, 2 months ago (2014-10-08 06:34:40 UTC) #31
jochen (gone - plz use gerrit)
On 2014/10/08 at 06:34:40, joleksy wrote: > On 2014/10/08 06:30:16, I haz the power (commit-bot) ...
6 years, 2 months ago (2014-10-08 14:58:00 UTC) #32
jln (very slow on Chromium)
On 2014/10/08 14:58:00, jochen wrote: > maybe the two CHECKS are a bit overkill, since ...
6 years, 2 months ago (2014-10-08 17:20:36 UTC) #33
joleksy
On 2014/10/08 17:20:36, jln wrote: > On 2014/10/08 14:58:00, jochen wrote: > > > maybe ...
6 years, 2 months ago (2014-10-15 07:39:18 UTC) #34
jochen (gone - plz use gerrit)
lgtm https://codereview.chromium.org/549303004/diff/80001/content/renderer/render_view_linux.cc File content/renderer/render_view_linux.cc (right): https://codereview.chromium.org/549303004/diff/80001/content/renderer/render_view_linux.cc#newcode96 content/renderer/render_view_linux.cc:96: prefs.default_font_size <= kMaxDefaultFontSize) nit add { }
6 years, 2 months ago (2014-10-15 15:27:40 UTC) #35
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/549303004/100001
6 years, 2 months ago (2014-10-16 10:02:23 UTC) #37
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_swarming/builds/24531)
6 years, 2 months ago (2014-10-16 11:06:22 UTC) #39
joleksy
On 2014/10/16 11:06:22, I haz the power (commit-bot) wrote: > Try jobs failed on following ...
6 years, 2 months ago (2014-10-16 12:48:54 UTC) #40
Elliot Glaysher
On 2014/10/16 12:48:54, joleksy wrote: > On 2014/10/16 11:06:22, I haz the power (commit-bot) wrote: ...
6 years, 2 months ago (2014-10-16 17:42:18 UTC) #41
joleksy
On 2014/10/16 17:42:18, Elliot Glaysher wrote: > On 2014/10/16 12:48:54, joleksy wrote: > > On ...
6 years, 2 months ago (2014-10-17 06:31:06 UTC) #43
joleksy
On 2014/10/17 06:31:06, joleksy wrote: > On 2014/10/16 17:42:18, Elliot Glaysher wrote: > > On ...
5 years, 10 months ago (2015-02-23 09:15:04 UTC) #45
dmazzoni
> The tests were added here: https://codereview.chromium.org/298653011 > > @dmazzoni, @plundblad: is it ok to ...
5 years, 10 months ago (2015-02-23 16:49:49 UTC) #46
joleksy
On 2015/02/23 16:49:49, dmazzoni wrote: > > The tests were added here: https://codereview.chromium.org/298653011 > > ...
5 years, 10 months ago (2015-02-24 07:50:26 UTC) #47
Peter Lundblad
On 2015/02/24 07:50:26, joleksy wrote: > On 2015/02/23 16:49:49, dmazzoni wrote: > > > The ...
5 years, 9 months ago (2015-03-16 11:43:35 UTC) #48
joleksy
On 2015/03/16 11:43:35, Peter Lundblad wrote: > Can you please disable the test instead of ...
5 years, 6 months ago (2015-06-26 14:22:53 UTC) #49
Elliot Glaysher
lgtm
5 years, 5 months ago (2015-07-06 20:40:13 UTC) #50
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/549303004/140001
5 years, 5 months ago (2015-07-07 06:11:53 UTC) #53
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_compile_dbg_32_ng/builds/70112) ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED, ...
5 years, 5 months ago (2015-07-07 06:13:33 UTC) #55
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/549303004/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/549303004/160001
5 years, 4 months ago (2015-08-21 09:35:57 UTC) #58
commit-bot: I haz the power
Try jobs failed on following builders: win8_chromium_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/builds/31188)
5 years, 4 months ago (2015-08-21 10:04:08 UTC) #60
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/549303004/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/549303004/180001
5 years, 4 months ago (2015-08-21 10:58:17 UTC) #63
commit-bot: I haz the power
Committed patchset #10 (id:180001)
5 years, 4 months ago (2015-08-21 13:10:15 UTC) #64
commit-bot: I haz the power
Patchset 10 (id:??) landed as https://crrev.com/f0bc1a926d8f0326ef38b8374e42bc8e244dd2ba Cr-Commit-Position: refs/heads/master@{#344746}
5 years, 4 months ago (2015-08-21 13:11:00 UTC) #65
kochi
On 2015/08/21 13:11:00, commit-bot: I haz the power wrote: > Patchset 10 (id:??) landed as ...
5 years, 3 months ago (2015-08-27 11:53:37 UTC) #66
kochi
5 years, 3 months ago (2015-08-27 11:57:46 UTC) #67
Message was sent while issue was closed.
A revert of this CL (patchset #10 id:180001) has been created in
https://codereview.chromium.org/1319613004/ by kochi@chromium.org.

The reason for reverting is: This caused a regression:
https://code.google.com/p/chromium/issues/detail?id=525176.

Powered by Google App Engine
This is Rietveld 408576698