RTL text using colored font is invisible with non-opaque text color

RESOLVED FIXED in Firefox 53

Status

()

Core
Graphics: Text
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: corbett.dav, Assigned: jfkthame)

Tracking

({regression})

50 Branch
mozilla54
regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox50 wontfix, firefox51+ wontfix, firefox52+ wontfix, firefox53+ fixed, firefox54 fixed)

Details

(Whiteboard: [gfx-noted], URL)

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20161115182233

Steps to reproduce:

1. I downloaded Amiri Quran Colored from https://github.com/khaledhosny/amiri-font/blob/eb309222e715615c872d625943a74c741646d1c7/amiri-quran-colored.ttf. This font uses COLR and CPAL tables to color the dots of Arabic letters red while leaving their skeletons in the text foreground color.
2. I rendered this document: data:text/html;charset=utf-8,<span style="font: 200px Amiri Quran Colored; color:%238888">(%D9%82)</span>


Actual results:

Between two #8888 parentheses was an invisible Arabic letter qaf.


Expected results:

Between two #8888 parentheses should be a #8888 Arabic letter qaf with opaque red dots.

For comparison, if you set the color to #888 or any other opaque color, it is rendered as expected.

Updated

2 years ago
Component: Untriaged → Graphics: Text
Product: Firefox → Core

Comment 1

2 years ago
Your thoughts, Jonathan?
Flags: needinfo?(jfkthame)

Comment 2

2 years ago
I can't reproduce this with FF 48 but got problem after FF 50 on mac.

Updated

2 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression, regressionwindow-wanted
Whiteboard: [gfx-noted]
One possible regression candidate would be:
https://hg.mozilla.org/mozilla-central/rev/e666f640f6434593a6077b4dc24e96bfdf868166
(but I didn't test!).

Comment 4

2 years ago
Quite possibly, but I’ve no idea what is going on. If I try to print the layer colors, I get:

r: 0.533333, g: 0.533333, b: 0.533333, a: 1.00000
r: 0.800000, g: 0.200000, b: 0.200000, a: 1.00000

So the first layer is getting the text color (since it has no assigned color in the font) and the second layer is getting the reddish color from the font. Not sure about the alpha values.

Comment 6

a year ago
Khaled, could you check this issue after your patch in bug 1283932.
status-firefox50: --- → affected
status-firefox51: --- → affected
status-firefox52: --- → affected
status-firefox53: --- → affected
tracking-firefox51: --- → ?
tracking-firefox52: --- → ?
tracking-firefox53: --- → ?
Flags: needinfo?(khaledhosny)
Track 51+/52+/53+ as regression.
tracking-firefox51: ? → +
tracking-firefox52: ? → +
tracking-firefox53: ? → +
(Assignee)

Comment 8

a year ago
Hmm, after considerable puzzling about this, I ran across an interesting quirk: the problem seems to be related to the colored Arabic font being RTL. If we override the character properties so that the entire testcase has LTR directionality, suddenly the Qaf appears:

data:text/html;charset=utf-8,<div style="unicode-bidi:bidi-override;"><span style="color:rgba(0,255,0,.5); font: 200px Amiri Quran Colored;">(%D9%82)</span>

I haven't yet identified exactly where things are going wrong, but this may provide a clue...
Flags: needinfo?(jfkthame)
status-firefox50: affected → wontfix
Version: 51 Branch → 50 Branch
Jonathan, any recent insights on this?
status-firefox51: affected → wontfix
status-firefox54: --- → affected
Flags: needinfo?(jfkthame)
(Assignee)

Comment 10

a year ago
Some additional testcases that may help narrow down the failure...

Similar failure with a color-emoji character (reproduced on macOS, but I expect it would fail similarly on other platforms provided there's a color emoji font):
  data:text/html;charset=utf-8,<div dir=rtl style="opacity:0.5; font:100px serif;">(%F0%9F%98%80)

Removing the dir=rtl attribute makes all the text appear as expected:
  data:text/html;charset=utf-8,<div style="opacity:0.5; font:100px serif;">(%F0%9F%98%80)

Removing the opacity:0.5 property makes all the text appear as expected:
  data:text/html;charset=utf-8,<div dir=rtl style="font:100px serif;">(%F0%9F%98%80)

Adding a strong-LTR character to the content makes all the text appear as expected:
  data:text/html;charset=utf-8,<div dir=rtl style="opacity:0.5; font:100px serif;">(%F0%9F%98%80x)

One key observation here is that it's not merely color-font + partial-opacity that leads to the failure; it is also necessary for the text involved to be RTL (either inherently, as in the Arabic example, or through the use of neutral-direction characters in an RTL context).

Even that isn't quite the whole story; in this example, the emoji (neutral) appear RTL because of the surrounding Arabic letters, yet the text still renders fine, because the surrounding parens are LTR:
  data:text/html;charset=utf-8,<div style="opacity:0.5; font:100px serif;">(%D9%81%F0%9F%98%80%F0%9F%98%81%D9%82)

Remove the parens, and it fails:
  data:text/html;charset=utf-8,<div style="opacity:0.5; font:100px serif;">%D9%81%F0%9F%98%80%F0%9F%98%81%D9%82
Flags: needinfo?(jfkthame)
(Assignee)

Comment 11

a year ago
Another note: with the testcases in comment 10, selecting the invisible text (e.g. with Cmd-A) makes it suddenly become visible. However, after deselecting it again, it may vanish as soon as a repaint is triggered (e.g. by resizing the window).
(Assignee)

Updated

a year ago
Summary: Colored font is invisible with non-opaque text color → RTL text using colored font is invisible with non-opaque text color
(Assignee)

Comment 12

a year ago
I have a patch that I think fixes this, just testing a bit before I post it for review.
Flags: needinfo?(khaledhosny)
(Assignee)

Updated

a year ago
Assignee: nobody → jfkthame
(Assignee)

Comment 13

a year ago
Created attachment 8839402 [details] [diff] [review]
Reftests for RTL rendering of color glyph with partial opacity (currently failing)
Attachment #8839402 - Flags: review?(jmuizelaar)
(Assignee)

Comment 14

a year ago
Created attachment 8839404 [details] [diff] [review]
Correct the bounds of the buffer used for drawing colored text with opacity when the run direction is RTL
Attachment #8839404 - Flags: review?(jmuizelaar)
(Assignee)

Comment 16

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b98ddcdbfdae76e5cd45decbc2249e5b7df837a
Bug 1318539 - Reftests for RTL rendering of color glyph with partial opacity (currently failing). r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/1c364b17c93ffb5745d283596bdc788bc028f675
Bug 1318539 - Correct the bounds of the buffer used for drawing colored text with opacity when the run direction is RTL. r=jrmuizel

Comment 17

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9b98ddcdbfda
https://hg.mozilla.org/mozilla-central/rev/1c364b17c93f
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox54: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Looks like a pretty small patch with tests included. Please request Aurora/Beta approval on this when you get a chance.
Flags: needinfo?(jfkthame)
Flags: in-testsuite+
(Assignee)

Comment 19

a year ago
Comment on attachment 8839404 [details] [diff] [review]
Correct the bounds of the buffer used for drawing colored text with opacity when the run direction is RTL

AFAICS this is not actually a regression from bug 1283932, as suggested earlier;  it has probably existed since color font support was initially implemented. The fix in bug 1283932 made the issue appear with the Amiri Colored font because previously, we didn't handle its color table at all, but this bug existed prior to that and could have been demonstrated with other fonts such as emoji in RTL content.

I don't think this is critical enough to justify a late-beta uplift; it's an extremely specific case that isn't likely to be common in the wild. But it would be good to take on Aurora, given that it is such a simple and small fix.

Approval Request Comment
[Feature/Bug causing the regression]: color font support

[User impact if declined]: RTL text using colored fonts (e.g. emoji) with partial opacity fails to render (is invisible)

[Is this code covered by automated tests?]: yes

[Has the fix been verified in Nightly?]: yes

[Needs manual test from QE? If yes, steps to reproduce]: no

[List of other uplifts needed for the feature/fix]: none

[Is the change risky?]: no

[Why is the change risky/not risky?]: localized change affecting only this particular code path (RTL + color-font or synthetic bold + transparency), so cannot regress any other case

[String changes made/needed]: none
Flags: needinfo?(jfkthame)
Attachment #8839404 - Flags: approval-mozilla-aurora?
wontfix for 52 per comment 19.  Thanks Jonathan.
status-firefox52: affected → wontfix
Doesn't apply to aurora
warning: conflicts while merging layout/reftests/text/reftest.list!
Flags: needinfo?(jfkthame)
(Assignee)

Comment 22

a year ago
(In reply to Sylvestre Ledru [:sylvestre] from comment #21)
> Doesn't apply to aurora
> warning: conflicts while merging layout/reftests/text/reftest.list!

The reftest patch (with tests annotated as "fails") lands first, then the actual bug-fix lands (and removes the annotations).
Flags: needinfo?(jfkthame)
Comment on attachment 8839404 [details] [diff] [review]
Correct the bounds of the buffer used for drawing colored text with opacity when the run direction is RTL

Fix an RTL text issue. Aurora53+.
Attachment #8839404 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Updated

a year ago
status-firefox53: affected → fixed
You need to log in before you can comment on or make changes to this bug.