1453451 - EmojiOne Money Bag (U+1F4B0) in emojione color font rendering incorrectly (due to stroke-width: context-value)

Status: RESOLVED WONTFIX

(Web Compatibility :: Site Reports, defect, P3)

Description

[(not currently active) Ted Mielczarek]

Attachment: Screenshot

Someone pointed this out on Twitter and I reproduced it locally. It turns out to be because we both had the EmojiOne Color font (https://github.com/eosrei/emojione-color-font) installed as a system font.

Testcase with EmojiOne Color as a web font: https://luser.github.io/emojione-money-bag-testcase/

The SVG source glyph looks sensible:
https://github.com/emojione/emojione/blob/2.2.7/assets/svg/1f4b0.svg

So this is either a bug in the font as converted from the SVG, or it's a bug in our font rendering.

Comment 1

[Jonathan Kew [:jfkthame]]

The webfont testcase renders OK in Edge, which tends to suggest it could be a bug on our side.

Comment 2

[(not currently active) Ted Mielczarek]

Good call, I forgot to check in Edge! Firefox has the same broken rendering on Linux/Windows/Mac here.

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

Regression window:

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=5f721a664bf64fed99184a866b60c24a6afcb3a0&tochange=62cebea1d1578461a423a9ca7848706455db9ea5

Looks like it appeared when stylo got enabled by default.

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

After playing a bit with the glyph in font environment, it seems that the reason is on stroke-width, which doesn't seem to have its default value "1", but something else. It is not yet clear what exactly is that value and where is it from.

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

If I specify `stroke-width="1"` on the root <svg> element, I can get the expected result, but if I add `stroke-width="context-value"` to the top level <g> element, the glyph would become broken again, so it seems the value is from context-value.

Another weird thing that, the context value isn't fixed related to SVG unit. When the font-size is 50px, `context-value` seems to be equivalent to "10", while when it's 100px, it's equivalent to "5", so it's probably a fixed pixel value somehow.

There are two questions, then:
* Why does stroke-width has a default value of context-value? Shouldn't its initial value be "1"?
* Where does the context value come from?

Comment 6

[Xidorn Quan [:xidorn] UTC+11]

OK, so for the first question, this is from the UA sheet of SVG[1]. Actually, when stylo is disabled, even specifying `stroke-width="context-value"` explicitly wouldn't cause this issue. So the problem seems to be related to where the context value is from.

[1] https://searchfox.org/mozilla-central/rev/14578d6f2d2ec5246572827061ecefa60a40855d/layout/svg/svg.css#32

Comment 7

[Xidorn Quan [:xidorn] UTC+11]

Attachment: svg glyph extracted from the font formatted

FWIW, the easiest way to play with this is:
1. go to roc's online tool: https://rocallahan.github.io/svg-opentype-workshop/
2. load a small font, e.g. Ahem, in the "Load file" field
3. click "Add document"
4. in the new textarea, past the content of this element
5. change the id attribute of <svg> to a small number, e.g. glyph4 is for "!"
6. optionally remove all unrelated characters from the preview area

Now you can play with the SVG and see what's going on with the glyph.

Comment 8

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #7)
> 4. in the new textarea, past the content of this element

I mean the content of this attachment.

Comment 9

[Xidorn Quan [:xidorn] UTC+11]

The spec of OpenType says[1]:
> context-value ... means the same as the computed value of the corresponding
> property of the context element, but scaled so that ...
but it doesn't seem to me stroke-width affects context-value in the SVG glyph at all.

jfkthame, do you have idea where does the value for context-value of stroke-width come from?


[1] https://docs.microsoft.com/zh-cn/typography/opentype/spec/svg#glyph-rendering

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Looking at:

  https://dxr.mozilla.org/mozilla-release/rev/54ee1aebd3927ea70f46398feffef7e869aae7e0/layout/style/nsRuleNode.cpp#9394

There's a couple things that make me suspicious about how this properties were handled on the old style system. In particular,  stroke-width: inherit; would reset context value to false, even though that struct is inherited and thus the value may be shared via other means.

Comment 11

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #10)
> There's a couple things that make me suspicious about how this properties
> were handled on the old style system. In particular,  stroke-width: inherit;
> would reset context value to false, even though that struct is inherited and
> thus the value may be shared via other means.

This seems like a bug of the old style system, but it's unrelated here, because this is effective only when `stroke-width: inherit` is specified explicitly. When it's not (like here), its value is inherited via duplicating the parent struct.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

FWIW I built mozilla-release to investigate this one, and with the old style system the context-value code isn't reached, which is surprising at least.

Comment 13

[Xidorn Quan [:xidorn] UTC+11]

Ah, reading that code again, it actually resets context-value when one is not specified! So, yes, context-value is never applied on an element in the old style system if it's not set explicitly, which is quite broken...

Comment 14

[Emilio Cobos Álvarez (:emilio)]

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #11)
> (In reply to Emilio Cobos Álvarez [:emilio] from comment #10)
> > There's a couple things that make me suspicious about how this properties
> > were handled on the old style system. In particular,  stroke-width: inherit;
> > would reset context value to false, even though that struct is inherited and
> > thus the value may be shared via other means.
> 
> This seems like a bug of the old style system, but it's unrelated here,
> because this is effective only when `stroke-width: inherit` is specified
> explicitly. When it's not (like here), its value is inherited via
> duplicating the parent struct.

Oh! No, it actually explains the behavior here, because that branch is also taken for eCSSUnit_Null (that is, no value specified).

So in the old style system the <path> elements didn't have the context-value bit, because they had other svg rules specified, but context-value was overriden to be false even though it should've been inherited.

The old behavior looks clearly bogus to me. Now, about how should these properties behave... From a style system point of view this is working correctly and it's producing the intended results.

Maybe context-value should be gone at computed-value time? If so, it's not clear to me how would we implement that in a non-intrusive way.

IMO if context-value is giving problems given we're the only ones really implementing them we should get rid of it. It also introduces a lot of complexity into the style system.

Xidorn, does the analysis above look right to you?

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Err, comment 14 mid-aired with comment 13, which is effectively the same conclusion.

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Ok, so I chatted a bit with Xidorn about this:

  https://mozilla.logbot.info/developers/20180421#c14647389

So to recap...

 * context-value was completely bogus in the old style system (see comment 13 / comment 14).
 * It adds complexity and a lot of otherwise unnecessary wrapper types to the style system.
 * Right now it's kept at computed-value time, and it seems like it really needs to be, since we don't know the context's scale at all at that time.
 * But that causes confusing semantics and wrong rendering (like this bug).

The spec handwaves how this value should be supported in:

 https://docs.microsoft.com/en-us/typography/opentype/spec/svg#glyph-rendering

Given that and that no other browser supports this, unless I'm missing something, I think we should remove context-value support.

Cam / Jonathan, do you have any take on this given the above?

Comment 17

[Xidorn Quan [:xidorn] UTC+11]

So... the problem here is that, the value is probably mean to be 1px, but the glyph has a transform="... scale(32.0)" (as can be seen from the attachment in comment 7), so that basically makes stroke 32px width...

I think another approach forward rather than unshipping context-value, is to remove the three context-value declarations from svg.css. That would still allow people to use context-value if they really need.

But that makes me wonder... how is context-value actually useful? According to my previous test, stroke-width on element doesn't seem to affect context-value at all, so it is basically just 1px in text rendering. Maybe it's useful for <canvas> drawing text?

Comment 18

[Jonathan Kew [:jfkthame]]

We have a basic testcase at layout/reftests/text-svgglyphs/svg-glyph-objectvalue.svg that seems to use this (and gives its "expected" result according to the reference file). But maybe the test was bogus, or at least insufficient?

Does Edge implement any of this? ISTR they have OpenType-SVG support, but haven't investigated the details.

If we want to consider dropping this, we should get feedback from others who were involved in developing the spec - in particular I think Adobe is using the OpenType-SVG format, though I don't know if they use context-value at all.

Comment 19

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Jonathan Kew (:jfkthame) from comment #18)
> We have a basic testcase at
> layout/reftests/text-svgglyphs/svg-glyph-objectvalue.svg that seems to use
> this (and gives its "expected" result according to the reference file). But
> maybe the test was bogus, or at least insufficient?

The SVG doc of that one is:
> <svg xmlns="http://www.w3.org/2000/svg">
> <!--
>     Test SVG glyphs for text object stroke value inheritance
>     Covers glyph ID range 68 (a) to 71 (d)
> -->
>   <!-- a -->
>   <rect x="100" y="-900" width="800" height="800" stroke="powderblue"
>     stroke-width="50" stroke-dashoffset="35"
>     stroke-dasharray="50 50" id="glyph68" />
> 
>   <!-- b -->
>   <rect x="100" y="-900" width="800" height="800" stroke="chartreuse"
>     stroke-width="50" stroke-dashoffset="35"
>     stroke-dasharray="context-value" id="glyph69" />
> 
>   <!-- c -->
>   <rect x="100" y="-900" width="800" height="800" stroke="sienna"
>     stroke-width="50" stroke-dasharray="50 50"
>     stroke-dashoffset="context-value" id="glyph70" />
> 
>   <!-- d -->
>   <rect x="100" y="-900" width="800" height="800" stroke="olivedrab"
>     stroke-width="context-value" stroke-dasharray="context-value"
>     stroke-dashoffset="context-value" id="glyph71" />
> </svg>
so it specifies a value for everything, which should work as expected regardless.

The issue we found is that, if stroke-width is not specified on an element, "context-value" wouldn't be inherited in the old style system (which is wrong), while it is inherited in stylo (which is correct).

Comment 20

[Xidorn Quan [:xidorn] UTC+11]

I would suggest we just remove the declarations from svg.css then. I assume context-value does have its use cases, and people who specifies it explicitly should understand what they are doing, but having it the default for stroke-width (and probably other two as well) is probably more a footgun...

Comment 21

[Jonathan Kew [:jfkthame]]

That sounds reasonable; but given that it is explicitly mentioned in https://www.w3.org/TR/SVG2/single-page.html#styling-UAStyleSheet (which in turn is referred to by https://docs.microsoft.com/en-us/typography/opentype/spec/svg#glyph-rendering), it's probably worth raising the question with the spec authors. (:heycam, wdyt?)

Comment 22

[Emilio Cobos Álvarez (:emilio)]

I don't know if anybody has data on how often this is used... According to http://stateofwebtype.com/#OpenType-SVG, Edge does support OpenType-SVG. If they don't look bogus on this test-case is because either:

 * They don't support context-value.
 * They don't set it for glyph documents.
 * They support it in a different way.

I think we should figure out what Edge does / plan to do here.

If they don't support context-value, I'd rather remove it, given how complex it is and the obvious lack of testing for the feature.

If they do support it, we should figure out what they do, in order to decide what's the best course for this feature, IMO, though Xidorn's proposal looks very reasonable.

Comment 23

[Emilio Cobos Álvarez (:emilio)]

Attachment: Edge rendering of svg-glyph-objectvalue.svg

So I uploaded this to a temporary location (https://crisal.io/tmp/svg-glyph-objectvalue.svg) along with the ref and the resource file, so I could see the rendering in Edge via browserstack.

Two observations:

 * The reftest doesn't pass on my machine at all (presumably because of HiDPI?). https://crisal.io/tmp/svg-glyph-objectvalue.svg is definitely different to https://crisal.io/tmp/svg-glyph-objectvalue-ref.svg.

 * The reftest doesn't render as intended at all in Edge (maybe an Edge bug?).

Comment 24

[Emilio Cobos Álvarez (:emilio)]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #23)
>  * The reftest doesn't pass on my machine at all (presumably because of
> HiDPI?). https://crisal.io/tmp/svg-glyph-objectvalue.svg is definitely
> different to https://crisal.io/tmp/svg-glyph-objectvalue-ref.svg.

Yeah, just verified that layout.css.devPixelsPerPx = 1 makes the test and ref render the same.

Comment 25

[Emilio Cobos Álvarez (:emilio)]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #22)
> I don't know if anybody has data on how often this is used... According to
> http://stateofwebtype.com/#OpenType-SVG, Edge does support OpenType-SVG. If
> they don't look bogus on this test-case is because either:
> 
>  * They don't support context-value.
>  * They don't set it for glyph documents.
>  * They support it in a different way.

Apparently[1] they use DirectWrite to render them. So the features are explained here[2], which doesn't include context-value at all.

I guess given there's no explicit decision about it it's a good idea for now just removing it from our stylesheets, and open a spec issue about it. I didn't find a way to file issues in the Microsoft page, so filed[3].

[1]: https://twitter.com/FremyCompany/status/987788726330839040
[2]: https://msdn.microsoft.com/en-us/library/windows/desktop/mt790715(v=vs.85).aspx
[3]: https://github.com/w3c/svgwg/issues/421

Comment 26

[Xidorn Quan [:xidorn] UTC+11]

So, for this specific case, I think Amelia's comment in the issue[1] makes sense that this is an issue of the font, which was hidden by the old style system bug, especially given the argument that not specifying stroke-width can lead to issue in other cases like referenced from <use> element.

Since this is (currently) just a single case of issue, and only happens in a relatively rarely used codepoint, and user needs to install the font themselves, I optimistically think that asking the font to fix it and deploy is faster and less risky then our changing the code to make it work and waiting for the change to propogate to release.


[1] https://github.com/w3c/svgwg/issues/421#issuecomment-383347782

Comment 27

[Cameron McCormack (:heycam)]

Without stroke-width:context-value, it's not possible to do anything useful with text strokes in SVG glyphs, and as stroking text becomes more useful (now that the CSSWG has decided to allow it in HTML content, and not just SVG content) there may be more use for this feature in the future.

I tend to agree with Xidorn that we should get the font fixed.

Comment 28

[Mike Taylor [:miketaylr]]

It looks like the font is EOL (due to upstream license changes): 

https://github.com/eosrei/emojione-color-font#project-end-of-life

Anybody wanna test if the successor project has the same issue? 

https://github.com/eosrei/twemoji-color-font

If so, we'll likely need to get the Twitter Emoji SVG set updated as well.

Comment 29

[Oana Arbuzov [:oanaarbuzov]]

The issue is still reproducible.
https://prnt.sc/10ozn0p

Tested with:
Browser / Version: Firefox Nightly 88.0a1 (2021-03-17)
Operating System: Windows 10 Pro

Comment 30

[Karl Dubost💡 :karlcow]

EOL is done. https://github.com/13rac1/emojione-color-font/issues/71

This project is an updated version.
https://github.com/EmojiTwo/emojitwo

After reading the full thread of comments and that the fact the font will not be fixed because EOL.
I don't think there is anything we can do on our side in webcompat.

And as there is no specific sites or system to address this. I will close it as WONTFIX. Feel free to move it around on where it needs to be fixed if it's in the code.

Comment 31

[Emilio Cobos Álvarez (:emilio)]

Yeah that's fine, thanks Karl!