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

RESOLVED FIXED in mozilla24

Status

()

Core
Layout: Text
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jfkthame, Assigned: jfkthame)

Tracking

unspecified
mozilla24
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Assignee)

Description

5 years ago
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).
(Assignee)

Comment 1

5 years ago
Created attachment 753614 [details] [diff] [review]
handle UTF-16 surrogate pairs in the SVG-in-OpenType glyphchar attribute
Attachment #753614 - Flags: review?(roc)
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+
(Assignee)

Comment 3

5 years ago
Created attachment 756151 [details] [diff] [review]
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.
Attachment #756151 - Flags: review?(roc)
(Assignee)

Updated

5 years ago
Attachment #756151 - Attachment is patch: true
Attachment #756151 - Attachment mime type: message/rfc822 → text/plain
Attachment #756151 - Flags: review?(roc) → review+
(Assignee)

Updated

5 years ago
Depends on: 878786
(Assignee)

Comment 4

5 years ago
Created attachment 757392 [details] [diff] [review]
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.)
Attachment #757392 - Flags: review?(roc)
Attachment #757392 - Flags: review?(roc) → review+
(Assignee)

Comment 5

5 years ago
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.
https://hg.mozilla.org/mozilla-central/rev/5df3bc40a38d
https://hg.mozilla.org/mozilla-central/rev/566576fe7088
https://hg.mozilla.org/mozilla-central/rev/1a88cc7dbd56
Status: NEW → RESOLVED
Last Resolved: 5 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.