Bug 141900

Summary: [iOS] Implement WTR::activateFonts() to fix missing fonts in WebKit2 test runs
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: Tools / TestsAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, dbates, mmaxfield, sam, simon.fraser, thorton
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch v1 mmaxfield: review+

David Kilzer (:ddkilzer)
Reported 2015-02-23 02:00:31 PST
Implement WTR::activateFonts() for iOS. The current ActivateFontsIOS.mm is just a placeholder, so fonts like Ahem are not being rendered properly when running tests. This fixes the fast/text/ruby-justification-flush.html reference test for iOS WebKit2 runs, and possibly others. The same test already passes with WebKit1 because the font activation code in DumpRenderTree already exists for iOS.
Attachments
Patch v1 (15.70 KB, patch)
2015-02-23 02:16 PST, David Kilzer (:ddkilzer)
mmaxfield: review+
David Kilzer (:ddkilzer)
Comment 1 2015-02-23 02:16:23 PST
Created attachment 247109 [details] Patch v1
Myles C. Maxfield
Comment 2 2015-02-23 12:37:40 PST
Comment on attachment 247109 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=247109&action=review > Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mmTools/WebKitTestRunner/InjectedBundle/mac/ActivateFonts.mm:275 > exit(1); > } I thought we actually linked the contents of these font files into a rodata section in the actual executable. Do we not do this any more? Does this work on iOS? > Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mmTools/WebKitTestRunner/InjectedBundle/mac/ActivateFonts.mm:278 > swizzleNSFontManagerMethods(); See https://bugs.webkit.org/show_bug.cgi?id=132159 for implementing the whitelist on iOS
David Kilzer (:ddkilzer)
Comment 3 2015-02-23 14:53:28 PST
(In reply to comment #2) > Comment on attachment 247109 [details] > Patch v1 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=247109&action=review > > > Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mmTools/WebKitTestRunner/InjectedBundle/mac/ActivateFonts.mm:275 > > exit(1); > > } > > I thought we actually linked the contents of these font files into a rodata > section in the actual executable. Do we not do this any more? Does this work > on iOS? This does work on iOS. See related code in DumpRenderTree. Dan Bernstein fixed this a while ago. > > Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mmTools/WebKitTestRunner/InjectedBundle/mac/ActivateFonts.mm:278 > > swizzleNSFontManagerMethods(); > > See https://bugs.webkit.org/show_bug.cgi?id=132159 for implementing the > whitelist on iOS Why is Bug 132159 OS X only? Why aren't we fixing both platforms at the same time?
Myles C. Maxfield
Comment 4 2015-02-23 16:00:07 PST
(In reply to comment #3) > (In reply to comment #2) > > Comment on attachment 247109 [details] > > Patch v1 > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=247109&action=review > > > > > Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mmTools/WebKitTestRunner/InjectedBundle/mac/ActivateFonts.mm:275 > > > exit(1); > > > } > > > > I thought we actually linked the contents of these font files into a rodata > > section in the actual executable. Do we not do this any more? Does this work > > on iOS? > > This does work on iOS. See related code in DumpRenderTree. Dan Bernstein > fixed this a while ago. > > > > Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mmTools/WebKitTestRunner/InjectedBundle/mac/ActivateFonts.mm:278 > > > swizzleNSFontManagerMethods(); > > > > See https://bugs.webkit.org/show_bug.cgi?id=132159 for implementing the > > whitelist on iOS > > Why is Bug 132159 OS X only? Why aren't we fixing both platforms at the > same time? Ultimately it's for both, but the initial goal of the patch is to do something which requires a CoreText whitelist. Baby steps.
David Kilzer (:ddkilzer)
Comment 5 2015-02-24 05:03:49 PST
Note You need to log in before you can comment on or make changes to this bug.