Bug 138891 - [OS X] Upright vertical text is completely broken for multi-code-unit code points
Summary: [OS X] Upright vertical text is completely broken for multi-code-unit code po...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks: 144303
  Show dependency treegraph
 
Reported: 2014-11-19 15:30 PST by Myles C. Maxfield
Modified: 2015-04-27 18:13 PDT (History)
6 users (show)

See Also:


Attachments
Patch (6.28 KB, patch)
2014-11-19 15:49 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (8.46 KB, patch)
2014-11-19 16:26 PST, Myles C. Maxfield
mitz: review+
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 2014-11-19 15:30:45 PST
Upright vertical text is completely broken for multi-code-unit codepoints
Comment 1 Myles C. Maxfield 2014-11-19 15:49:01 PST
Created attachment 241899 [details]
Patch
Comment 2 Myles C. Maxfield 2014-11-19 15:49:32 PST
<rdar://problem/18421386>
Comment 3 mitz 2014-11-19 16:07:47 PST
Comment on attachment 241899 [details]
Patch

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

> Source/WebCore/platform/graphics/mac/GlyphPageTreeNodeMac.cpp:153
> -                            setGlyphDataForIndex(offset + stringIndices[i], glyphs[i], fontData);
> +                            // Because of the size of the GlyphPage, the string will never straddle a code-unit size boundary
> +                            unsigned index = U16_IS_LEAD(buffer[stringIndices[i]]) ? stringIndices[i] / 2 : stringIndices[i];
> +                            setGlyphDataForIndex(offset + index, glyphs[i], fontData);

Shouldn’t you make this change in the else block below as well?

It’s weird to write the logic this way, because we are relying on the fact that buffer is either uniformly BMP characters or surrogate pairs. It wouldn’t work otherwise.

I think that instead you should use the glyphStep approach we use above.
Comment 4 Myles C. Maxfield 2014-11-19 16:26:08 PST
Created attachment 241905 [details]
Patch
Comment 5 Myles C. Maxfield 2014-11-19 17:35:47 PST
http://trac.webkit.org/changeset/176372