Bug 145678

Summary: [iOS] Emoji overlap preceding lines
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, darin, enrica, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Archive of layout-test-results from ews100 for mac-mavericks
none
Archive of layout-test-results from ews104 for mac-mavericks-wk2
none
Patch enrica: review+

Description Myles C. Maxfield 2015-06-04 18:14:58 PDT
[iOS] Emoji overlap preceeding lines
Comment 1 Myles C. Maxfield 2015-06-04 18:37:09 PDT
Created attachment 254329 [details]
Patch
Comment 2 Myles C. Maxfield 2015-06-04 18:37:39 PDT
<rdar://problem/10684914>
Comment 3 Myles C. Maxfield 2015-06-04 19:23:06 PDT
Created attachment 254331 [details]
Patch
Comment 4 Build Bot 2015-06-04 19:57:05 PDT
Comment on attachment 254331 [details]
Patch

Attachment 254331 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6167040318504960

New failing tests:
fast/text/emoji.html
Comment 5 Build Bot 2015-06-04 19:57:09 PDT
Created attachment 254332 [details]
Archive of layout-test-results from ews100 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 6 Build Bot 2015-06-04 20:01:45 PDT
Comment on attachment 254331 [details]
Patch

Attachment 254331 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5770894412611584

New failing tests:
fast/text/emoji.html
Comment 7 Build Bot 2015-06-04 20:01:50 PDT
Created attachment 254333 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 8 Myles C. Maxfield 2015-06-04 20:30:02 PDT
Created attachment 254336 [details]
Patch
Comment 9 Darin Adler 2015-06-05 11:05:00 PDT
Comment on attachment 254336 [details]
Patch

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

> Source/WebCore/platform/graphics/cocoa/FontCocoa.mm:-275
> -        m_fontMetrics.setLineSpacing(0);

Maybe the real intention here was to set the line spacing to m_platformData.size()?
Comment 10 Myles C. Maxfield 2015-06-05 11:26:04 PDT
Comment on attachment 254336 [details]
Patch

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

>> Source/WebCore/platform/graphics/cocoa/FontCocoa.mm:-275
>> -        m_fontMetrics.setLineSpacing(0);
> 
> Maybe the real intention here was to set the line spacing to m_platformData.size()?

I tried that, but that value was too low to actually solve the problem.
Comment 11 Myles C. Maxfield 2015-06-05 11:39:53 PDT
I talked with Enrica, and we came up with some interesting points.

1. The line-height of the lines inside a block is calculated to be the maximum of the line-heights of the constituent fonts. This means that, if the emoji's line height is 0, emoji won't affect the line spacing of a paragraph that it's within.

2. It is possible to trigger this bug if the natural line-height of the font used for the surrounding text is significantly smaller than the natural line height of the emoji font, but in practice, this seems rare.

2. Assuming #2 does not occur, the bug does not present itself if there are any non-emoji characters which are siblings of the emoji (because of the maximum line-height rule described in #1).

3. Assuming #2 does not occur, the bug only presents itself if the block is styled with "font-family: 'Apple Color Emoji' (, other fallback fonts)". This is a structure which Apple's Mail client should never emit.
Comment 12 Myles C. Maxfield 2015-06-05 11:57:39 PDT
(In reply to comment #11)
> I talked with Enrica, and we came up with some interesting points.
> 
> 1. The line-height of the lines inside a block is calculated to be the
> maximum of the line-heights of the constituent fonts. This means that, if
> the emoji's line height is 0, emoji won't affect the line spacing of a
> paragraph that it's within.
> 
> 2. It is possible to trigger this bug if the natural line-height of the font
> used for the surrounding text is significantly smaller than the natural line
> height of the emoji font, but in practice, this seems rare.
> 
> 2. Assuming #2 does not occur, the bug does not present itself if there are
> any non-emoji characters which are siblings of the emoji (because of the
> maximum line-height rule described in #1).
> 
> 3. Assuming #2 does not occur, the bug only presents itself if the block is
> styled with "font-family: 'Apple Color Emoji' (, other fallback fonts)".
> This is a structure which Apple's Mail client should never emit.

Note that, related to the last item, the font that causes this bug is Apple-specific, yet Apple's mail client won't emit markup which triggers this. This means that the bug is rare.
Comment 13 Myles C. Maxfield 2015-06-05 11:58:04 PDT
<rdar://problem/21263287>
Comment 14 Enrica Casucci 2015-06-05 14:36:11 PDT
Comment on attachment 254336 [details]
Patch

I discussed this with Myles at length. Setting lineSpacing to 0 was used in earlier versions of WebKit for iOS and it is no longer needed.
Comment 15 Myles C. Maxfield 2015-06-05 14:43:54 PDT
(In reply to comment #14)
> Comment on attachment 254336 [details]
> Patch
> 
> I discussed this with Myles at length. Setting lineSpacing to 0 was used in
> earlier versions of WebKit for iOS and it is no longer needed.

And Dan chimed in!
Comment 16 Myles C. Maxfield 2015-06-05 14:46:53 PDT
Committed r185266: <http://trac.webkit.org/changeset/185266>