Chromium Code Reviews Chromium Code Reviews
Issue
1276003003:
Reland 2: mac: Use a placeholder string for the family name of the system font. (Closed)
DescriptionReland 2: mac: Use a placeholder string for the family name of the system font.
This CL contains 4 functional differences from Reland #1.
1) The default system font average char width hack is only applied on OSX 10.9
or earlier. The 10.10 layout tests were recently rebaselined to not use this
hack.
2) On OSX 10.10+, the implementation of MatchNSFontFamily uses the new NSFont
private method +[NSFont fontWithSize:weight:]. This matches mmaxfield@'s
implementation for WebKit.
3) The first reland was accidentally comparing an AppKit font weight with
blink::FontWeight.
4) The first reland forgot to apply traits to the default system font.
This CL rebaselines 5 Yosemite layout tests that use a bold system font. The
previous logic in MatchNSFontFamily was converting the default system font to a
string, and then back to an NSFont*. On Yosemite, this operation produces
non-deterministic results (dependent on global state). The new mechanism for
grabbing the system font is deterministic.
Original issue's description:
> mac: Use a placeholder string for the family name of the system font.
>
> This CL is inspired by mmaxfield@apple.com and
> http://trac.webkit.org/changeset/184353.
>
> The system font varies by OS version, as well as the version of the OSX SDK that
> the binary is compiled against. This CL uses a placeholder string for the family
> name of the system font that is passed around.
>
> This CL has two positive effects.
>
> 1) Logic in LayoutTestControl* was checking the family name of the font directly
> against the string "Lucida Grande". This was broken any time the system font
> wasn't "Lucida Grande".
> 2) Layout tests were failing on OSX 10.10, 10.11, and when the binary was
> compiled against the OSX 10.10 SDK. This CL fixes this particular set of
> problems by always using the font "Lucida Grande" for layout tests.
>
> This CL has one non-regression behavior change. If the font family of a text
> control is set to "Lucida Grande", its width is no longer modified by the
> special logic in LayoutTestControl*. This isn't a regression because there is no
> reason to expect the special logic of LayoutTestControl to apply to the text
> control to begin with (since "Lucida Grande" may or may not be the system font).
> If we really want the system font to always have this behavior, we should
> implement font family keywords like "-apple-system". See 515989 for more
> details.
>
> BUG=515989, 515836
BUG=515989, 515836
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200304
Patch Set 1 : Copy of Reland 1. #Patch Set 2 : Don't perform average char width hack on 10.10+. #Patch Set 3 : Use new font weight methods on Yosemite. Add unit test. #Patch Set 4 : Add version utils. #Patch Set 5 : Add platform export. #Patch Set 6 : Compile errors. #Patch Set 7 : Remove unused code. #Patch Set 8 : Rebase against top of tree. #Patch Set 9 : Rebaseline Yosemite tests that use bold fonts. #
Total comments: 4
Patch Set 10 : Comments from tkent. #
Total comments: 4
Patch Set 11 : Comments from tkent, round two. #Patch Set 12 : Rebase against top of tree. Comment out more specialized, duplicate rebaseline. #Messages
Total messages: 30 (12 generated)
|
