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

Issue 2838003: Map Liberation Sans Mono and Ascender Sans Mono to Courier New.... (Closed)

Created:
10 years, 6 months ago by jungshik at Google
Modified:
9 years, 7 months ago
Reviewers:
Evan Martin
CC:
chromium-reviews, jlees
Visibility:
Public.

Description

Map Liberation Sans Mono and Ascender Sans Mono to Courier New. They're metrically compatible with each other, but the former is sans serif (monospace) while ther latter is serif (monospace), which is why we didn't include this mapping in the two previous CLs (one by Evan and the other by me). However, on ChromeOS, we use 'Ascender Sans Mono' as a substitute for Courier New so that they have to be regarded as 'identical'. Without this change, a text node styled with 'Courier New' is not rendered with our replacement font while a text node styled with 'Courier New, momospace' or 'monospace' is rendered with our replacement font. BUG=cros:4006 TEST=See the bug. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=49975

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -4 lines) Patch
M skia/ext/SkFontHost_fontconfig_direct.cpp View 3 chunks +12 lines, -4 lines 2 comments Download

Messages

Total messages: 2 (0 generated)
jungshik at Google
10 years, 6 months ago (2010-06-14 21:04:42 UTC) #1
Evan Martin
10 years, 6 months ago (2010-06-14 23:08:55 UTC) #2
LGTM without the liberation mono bit.

http://codereview.chromium.org/2838003/diff/1/2
File skia/ext/SkFontHost_fontconfig_direct.cpp (right):

http://codereview.chromium.org/2838003/diff/1/2#newcode56
skia/ext/SkFontHost_fontconfig_direct.cpp:56: // Arial, Times New Roman and
Courier New  with a character repertoire
nit: you have an extra space before "with"

http://codereview.chromium.org/2838003/diff/1/2#newcode69
skia/ext/SkFontHost_fontconfig_direct.cpp:69: strcasecmp(fontname, "Liberation
Sans Mono") == 0 ||
I don't think we want to do this.

If you have:
  font-family: Courier New, monospace;
and the user configured some *other* font as their monospace font, this will
cause them to get Liberation Sans over their preference.

I think what we want to do is to map Courier New to a monospace-family font in
WebKit such that for "font-family: Courier New;" WebKit knows that it should try
the default monospace font in the fallback case.

Since Ascender is aesthetically compatible, I think that substitution is ok
though -- my objection is only to using Liberation here.

Powered by Google App Engine
This is Rietveld 408576698