875629 - SVG-in-OpenType glyphchar attribute does not accept non-BMP characters

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Jonathan Kew [:jfkthame]]

The glyphchar attribute doesn't work for supplementary-plane characters, as gfxSVGGlyphsDocument::InsertGlyphChar just uses the individual (UTF16) codepoints in the string, without checking for and interpreting surrogate pairs.

Unfortunately, the Unicode emoji characters - which are the most obvious use-case for the SVG-glyphs feature - are encoded in plane 15 (except a few that were unified with existing symbols).

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: handle UTF-16 surrogate pairs in the SVG-in-OpenType glyphchar attribute

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 753614 [details] [diff] [review]
handle UTF-16 surrogate pairs in the SVG-in-OpenType glyphchar attribute

Review of attachment 753614 [details] [diff] [review]:
-----------------------------------------------------------------

Please add a testcase! Especially for the non-BMP UVS!

::: gfx/thebes/gfxSVGGlyphs.cpp
@@ +414,5 @@
> +    NS_ASSERTION(aPos < len, "already at end of string");
> +
> +    uint32_t c1 = aString[aPos++];
> +    if (NS_IS_HIGH_SURROGATE(c1)) {
> +        NS_ASSERTION(aPos < len, "trailing high surrogate");

Add a comment explaining that the UTF8/XML parsing should have not have allowed a lone high surrogate to be produced.

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: reftest for use of supplementary-plane codepoint in glyphchar

The test here uses a pair of fonts that have the same SVG glyph, one of them where it's encoded at its correct supplementary-plane location, and another where it's assigned to a BMP character; then we can check that the two render identically.

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: reftest for SVG glyph encoded using a supplementary-plane variation sequence

The test font here includes two copies of the "smiling cat face" SVG glyph, one at its correct Unicode value of U+1f63b, and the other encoded using the variation sequence U+1f431,U+e0100, so we can test that they both render the same. (Note that this glyph uses embedded <image> elements, so it also serves as a testcase for bug 878786.)

Comment 5

[Jonathan Kew [:jfkthame]]

Pushed patch and testcases:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5df3bc40a38d
https://hg.mozilla.org/integration/mozilla-inbound/rev/566576fe7088
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a88cc7dbd56

Note that while I have confirmed locally that the reftests work as expected, they will not actually be running on m-c yet, until bug 871961 is (re-)resolved.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5df3bc40a38d
https://hg.mozilla.org/mozilla-central/rev/566576fe7088
https://hg.mozilla.org/mozilla-central/rev/1a88cc7dbd56