Bug 200065 - REGRESSION(r241288): Text on Yahoo Japan mobile looks too bold
Summary: REGRESSION(r241288): Text on Yahoo Japan mobile looks too bold
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks: 200047
  Show dependency tree / graph
 
Reported: 2019-07-23 17:25 PDT by Myles C. Maxfield
Modified: 2019-07-30 14:48 PDT (History)
12 users (show)

See Also:

Attachments
Patch (7.88 KB, patch)
2019-07-23 17:34 PDT, Myles C. Maxfield 		no flags Details | Formatted Diff | Diff
Patch (9.05 KB, patch)
2019-07-29 19:18 PDT, Myles C. Maxfield 		no flags Details | Formatted Diff | Diff
Patch (9.08 KB, patch)
2019-07-29 20:58 PDT, Myles C. Maxfield 		simon.fraser: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2019-07-23 17:25:12 PDT 
REGRESSION(r241288): Text on Yahoo Japan mobile looks too bold
Comment 1 Myles C. Maxfield 2019-07-23 17:34:40 PDT 
Created attachment 374741 [details]
Patch
Comment 2 Myles C. Maxfield 2019-07-23 17:34:43 PDT 
<rdar://problem/50912757>
Comment 3 Simon Fraser (smfr) 2019-07-23 19:08:42 PDT 
Comment on attachment 374741 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=374741&action=review

> Source/WTF/wtf/Platform.h:1619
> +#if (PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 130000) || (PLATFORM(WATCHOS) && __WATCH_OS_VERSION_MIN_REQUIRED >= 60000) || (PLATFORM(APPLETV) && __TV_OS_VERSION_MIN_REQUIRED >= 130000)
> +#define USE_HIRAGINO_SANS_WORKAROUND 1
> +#endif

Seems wrong for something so specific to be in Platform.h

> Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:1156
> +    // This is yucky code, but it's necessary for yahoo.co.jp.
> +    // FIXME: https://bugs.webkit.org/show_bug.cgi?id=200047 Remove this code.

Should this then be a Quirk?
Comment 4 Alexey Proskuryakov 2019-07-23 23:09:45 PDT 
> Seems wrong for something so specific to be in Platform.h

What would you suggest? My understanding is that Platform.h is exactly where such things need to be. 

Seems like we want to be checking for PLATFORM(IOS) not PLATFORM(IOS_FAMILY) here.
Comment 5 Simon Fraser (smfr) 2019-07-24 11:15:59 PDT 
Comment on attachment 374741 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=374741&action=review

>> Source/WTF/wtf/Platform.h:1619
>> +#endif
> 
> Seems wrong for something so specific to be in Platform.h

Would this stay in the tree in perpetuity? When would we know that we can remove it?
Comment 6 Myles C. Maxfield 2019-07-29 12:03:01 PDT 
Hooking this up to a quirk appears to be fairly difficult, since control flows through a singleton FontDatabase object which owns all the CTFonts. Instead, I'll try to modify the request rather than modify the font object.
Comment 7 Myles C. Maxfield 2019-07-29 19:18:13 PDT 
Created attachment 375139 [details]
Patch
Comment 8 Myles C. Maxfield 2019-07-29 20:58:06 PDT 
Created attachment 375144 [details]
Patch
Comment 9 Simon Fraser (smfr) 2019-07-30 11:20:59 PDT 
Comment on attachment 375144 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=375144&action=review

> Source/WebCore/page/Quirks.cpp:396
> +    // FIXME: https://bugs.webkit.org/show_bug.cgi?id=200047 Remove this quirk.

Nicer to use webkit.org/b/200047
Comment 10 Myles C. Maxfield 2019-07-30 14:48:34 PDT 
Committed r248018: <https://trac.webkit.org/changeset/248018>