Bug 172893 - [macOS] System emoji font is not directly specifiable in the font-family property
Summary: [macOS] System emoji font is not directly specifiable in the font-family prop...
Status: RESOLVED WONTFIX
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: 172886
Blocks:
  Show dependency treegraph
 
Reported: 2017-06-03 01:53 PDT by Myles C. Maxfield
Modified: 2017-06-03 22:02 PDT (History)
9 users (show)

See Also:


Attachments
Patch (33.18 KB, patch)
2017-06-03 02:25 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (33.28 KB, patch)
2017-06-03 02:35 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-elcapitan (978.67 KB, application/zip)
2017-06-03 03:40 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (1.14 MB, application/zip)
2017-06-03 03:45 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (12.27 MB, application/zip)
2017-06-03 04:08 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews117 for mac-elcapitan (1.73 MB, application/zip)
2017-06-03 04:42 PDT, Build Bot
no flags Details
Patch (34.72 KB, patch)
2017-06-03 10:38 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (26.88 KB, patch)
2017-06-03 11:25 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (60.45 KB, patch)
2017-06-03 15:17 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (60.55 KB, patch)
2017-06-03 15:20 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (61.02 KB, patch)
2017-06-03 15:33 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (61.03 KB, patch)
2017-06-03 16:32 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2017-06-03 01:53:16 PDT
[macOS] System emoji font is not accessible
Comment 1 Myles C. Maxfield 2017-06-03 02:25:13 PDT
Created attachment 311908 [details]
Patch
Comment 2 Myles C. Maxfield 2017-06-03 02:35:34 PDT
Created attachment 311909 [details]
Patch
Comment 3 Build Bot 2017-06-03 03:40:31 PDT
Comment on attachment 311909 [details]
Patch

Attachment 311909 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3864950

New failing tests:
fast/text/system-emoji-font.html
Comment 4 Build Bot 2017-06-03 03:40:33 PDT
Created attachment 311910 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 5 Build Bot 2017-06-03 03:45:31 PDT
Comment on attachment 311909 [details]
Patch

Attachment 311909 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3864954

New failing tests:
fast/text/system-emoji-font.html
Comment 6 Build Bot 2017-06-03 03:45:32 PDT
Created attachment 311911 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 7 Build Bot 2017-06-03 04:08:50 PDT
Comment on attachment 311909 [details]
Patch

Attachment 311909 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3864973

New failing tests:
fast/text/system-emoji-font.html
Comment 8 Build Bot 2017-06-03 04:08:52 PDT
Created attachment 311913 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 9 Build Bot 2017-06-03 04:42:42 PDT
Comment on attachment 311909 [details]
Patch

Attachment 311909 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3865126

New failing tests:
fast/text/system-emoji-font.html
Comment 10 Build Bot 2017-06-03 04:42:44 PDT
Created attachment 311914 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 11 Myles C. Maxfield 2017-06-03 10:38:04 PDT
Created attachment 311932 [details]
Patch
Comment 12 Myles C. Maxfield 2017-06-03 10:43:58 PDT
<rdar://problem/31639090>
Comment 13 Myles C. Maxfield 2017-06-03 11:25:51 PDT
Created attachment 311936 [details]
Patch
Comment 14 Sam Weinig 2017-06-03 12:35:23 PDT
Given we expose the system font to the web, why wouldn't we want to expose the system emoji font to the web?
Comment 15 Simon Fraser (smfr) 2017-06-03 12:50:46 PDT
Comment on attachment 311936 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        [macOS] System emoji font is not accessible

In the accessibility sense or the "can't be used by web pages" sense?
Comment 16 Sam Weinig 2017-06-03 13:19:13 PDT
Why can't this happen automatically if the font specified is -apple-system or system-ui (also maybe -apple-system-monospaced-numbers)?
Comment 17 Myles C. Maxfield 2017-06-03 14:43:34 PDT
(In reply to Sam Weinig from comment #14)
> Given we expose the system font to the web, why wouldn't we want to expose
> the system emoji font to the web?

I haven't received any requests from web authors to make this available on the Web. Because this is being created specifically for a native app, it seems best to be cautious. I'd prefer not to add to the surface area of things we need to support forever, if it's possible not to.

If you think we should make this available on the Web, I'd be happy to do that.

(In reply to Sam Weinig from comment #16)
> Why can't this happen automatically if the font specified is -apple-system
> or system-ui (also maybe -apple-system-monospaced-numbers)?

It does happen automatically in this situation. However, the cost is a round trip to fontd for each emoji character to be looked up. The goal of this patch is to improve performance by adding a keyword to the native app's font fallback list.

Perhaps this patch should include a performance test.
Comment 18 Myles C. Maxfield 2017-06-03 14:44:32 PDT
(In reply to Myles C. Maxfield from comment #17)
> (In reply to Sam Weinig from comment #14)
> > Given we expose the system font to the web, why wouldn't we want to expose
> > the system emoji font to the web?
> 
> I haven't received any requests from web authors to make this available on
> the Web. Because this is being created specifically for a native app, it
> seems best to be cautious. I'd prefer not to add to the surface area of
> things we need to support forever, if it's possible not to.
> 
> If you think we should make this available on the Web, I'd be happy to do
> that.
> 
> (In reply to Sam Weinig from comment #16)
> > Why can't this happen automatically if the font specified is -apple-system
> > or system-ui (also maybe -apple-system-monospaced-numbers)?
> 
> It does happen automatically in this situation. However, the cost is a round
> trip to fontd for each emoji character to be looked up. The goal of this
> patch is to improve performance by adding a keyword to the native app's font
> fallback list.
> 
> Perhaps this patch should include a performance test.

I'll upload a new patch with a performance test that describes this stuff in the ChangeLog (as well as making the title of the bug more clear).
Comment 19 Myles C. Maxfield 2017-06-03 15:17:21 PDT
Created attachment 311945 [details]
Patch
Comment 20 Myles C. Maxfield 2017-06-03 15:20:05 PDT
Created attachment 311946 [details]
Patch
Comment 21 Myles C. Maxfield 2017-06-03 15:22:17 PDT
(In reply to Myles C. Maxfield from comment #17)
> (In reply to Sam Weinig from comment #14)
> > Given we expose the system font to the web, why wouldn't we want to expose
> > the system emoji font to the web?
> 
> I haven't received any requests from web authors to make this available on
> the Web. Because this is being created specifically for a native app, it
> seems best to be cautious. I'd prefer not to add to the surface area of
> things we need to support forever, if it's possible not to.
> 
> If you think we should make this available on the Web, I'd be happy to do
> that.

Another reason is that, once I implement system-ui correctly (aka honoring the Core Text font fallback mechanism inside the system font), the need for this will go away. However, implementing this fallback correctly shouldn't be done now.
Comment 22 Myles C. Maxfield 2017-06-03 15:33:29 PDT
Created attachment 311947 [details]
Patch
Comment 23 Sam Weinig 2017-06-03 15:36:47 PDT
(In reply to Myles C. Maxfield from comment #17)
> (In reply to Sam Weinig from comment #14)
> > Given we expose the system font to the web, why wouldn't we want to expose
> > the system emoji font to the web?
> 
> I haven't received any requests from web authors to make this available on
> the Web. Because this is being created specifically for a native app, it
> seems best to be cautious. I'd prefer not to add to the surface area of
> things we need to support forever, if it's possible not to.
> 
> If you think we should make this available on the Web, I'd be happy to do
> that.
> 
> (In reply to Sam Weinig from comment #16)
> > Why can't this happen automatically if the font specified is -apple-system
> > or system-ui (also maybe -apple-system-monospaced-numbers)?
> 
> It does happen automatically in this situation. However, the cost is a round
> trip to fontd for each emoji character to be looked up. The goal of this
> patch is to improve performance by adding a keyword to the native app's font
> fallback list.
> 
> Perhaps this patch should include a performance test.

This seems like a bad solution. We should not make each application know they need to do this just to get better performance. Perhaps you can explain more clearly why this needs to be specified by the application and can't happen automatically.
Comment 24 Myles C. Maxfield 2017-06-03 16:01:36 PDT
(In reply to Sam Weinig from comment #23)
> (In reply to Myles C. Maxfield from comment #17)
> > (In reply to Sam Weinig from comment #14)
> > > Given we expose the system font to the web, why wouldn't we want to expose
> > > the system emoji font to the web?
> > 
> > I haven't received any requests from web authors to make this available on
> > the Web. Because this is being created specifically for a native app, it
> > seems best to be cautious. I'd prefer not to add to the surface area of
> > things we need to support forever, if it's possible not to.
> > 
> > If you think we should make this available on the Web, I'd be happy to do
> > that.
> > 
> > (In reply to Sam Weinig from comment #16)
> > > Why can't this happen automatically if the font specified is -apple-system
> > > or system-ui (also maybe -apple-system-monospaced-numbers)?
> > 
> > It does happen automatically in this situation. However, the cost is a round
> > trip to fontd for each emoji character to be looked up. The goal of this
> > patch is to improve performance by adding a keyword to the native app's font
> > fallback list.
> > 
> > Perhaps this patch should include a performance test.
> 
> This seems like a bad solution. We should not make each application know
> they need to do this just to get better performance. Perhaps you can explain
> more clearly why this needs to be specified by the application and can't
> happen automatically.

I could special-case the combination of system-ui and emoji characters in lookupFallbackFont(), but it would still require some opt-in because semantics would be changed. Any characters that exist in both the emoji font as well as any other item in the Core Text fallback list (like ♀ and ♂) would get a different font.

Or, I could guard this special-case on characters that I know are only in the emoji font, but I don't think we want to be in the business of listing characters that have to match another part of the system.

Doing this special-casing would effectively be moving the emoji font to the top of the fallback list (to just behind San Francisco). It seems to me that, if the client is going to have to do some opt-in, listing the emoji font in the font-family list is the most descriptive way of describing what is going on.

If you think these other alternatives are better, I'd be happy to implement them.
Comment 25 Myles C. Maxfield 2017-06-03 16:32:25 PDT
Created attachment 311950 [details]
Patch
Comment 26 Sam Weinig 2017-06-03 18:03:21 PDT
(In reply to Myles C. Maxfield from comment #24)
> Doing this special-casing would effectively be moving the emoji font to the
> top of the fallback list (to just behind San Francisco). It seems to me
> that, if the client is going to have to do some opt-in, listing the emoji
> font in the font-family list is the most descriptive way of describing what
> is going on.

My point is that the client should not need to opt in. It seems you mentioned the solution that would work without requiring an opt in, "honoring the Core Text font fallback mechanism inside the system font". If that is the correct solution, we should do that. It doesn't seem like a good tradeoff to add SPI for a performance problem we can solve in our own code.
Comment 27 Myles C. Maxfield 2017-06-03 22:02:00 PDT
(In reply to Sam Weinig from comment #26)
> (In reply to Myles C. Maxfield from comment #24)
> > Doing this special-casing would effectively be moving the emoji font to the
> > top of the fallback list (to just behind San Francisco). It seems to me
> > that, if the client is going to have to do some opt-in, listing the emoji
> > font in the font-family list is the most descriptive way of describing what
> > is going on.
> 
> My point is that the client should not need to opt in. It seems you
> mentioned the solution that would work without requiring an opt in,
> "honoring the Core Text font fallback mechanism inside the system font". If
> that is the correct solution, we should do that. It doesn't seem like a good
> tradeoff to add SPI for a performance problem we can solve in our own code.

Okay, I'll work on that instead.