Closed Bug 875629 Opened 8 years ago Closed 8 years ago

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


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

Not set





(Reporter: jfkthame, Assigned: jfkthame)




(3 files)

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 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.
Attachment #753614 - Flags: review?(roc) → review+
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.
Attachment #756151 - Flags: review?(roc)
Attachment #756151 - Attachment is patch: true
Attachment #756151 - Attachment mime type: message/rfc822 → text/plain
Depends on: 878786
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.)
Attachment #757392 - Flags: review?(roc)
Pushed patch and testcases:

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.
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.