Some combinations of Hebrew vowels and consonants not displayed in certain fonts with harfbuzz

RESOLVED FIXED in Firefox 13

Status

()

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

People

(Reporter: smontagu, Assigned: jfkthame)

Tracking

({regression})

Trunk
mozilla15
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox13+ verified, firefox14+ verified)

Details

(Whiteboard: [qa+])

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 625471 [details]
Testcase

Some combinations of Hebrew consonants and vowels are displayed as empty rectangles in some fonts.

Out of the fonts installed on my Linux system I see this with יִ (U+05D9 U+05B4) in Tahoma, Miriam and Miriam Fixed; and with many more combinataions in Linux Libertine and Linux Biolinum. 

If I turn off displaying Hebrew with harfbuzz in about:config, the bug disappears.
(Reporter)

Comment 1

5 years ago
Regression range is http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=be559203ece8&tochange=373c710112e6, which includes bug 728866, and looking at the cases which mis-display with Linux Libertine, they seem to exactly match the combinations handled in attachment 598849 [details] [diff] [review].
Blocks: 728866
Keywords: regression
(Assignee)

Comment 2

5 years ago
Created attachment 625602 [details] [diff] [review]
patch, don't let harfbuzz apply [de]composition if the target chars aren't supported in the font

The problem isn't limited to Hebrew; it could affect any case where harfbuzz (potentially) applies Unicode composition or decomposition during rendering, because our HBGetGlyph callback always returns TRUE rather than correctly reporting whether the glyph was actually found.

(This didn't matter before harfbuzz normalization was implemented; because we do per-character font matching, we already know that all the characters we're asking harfbuzz to shape are in fact supported by the font. But for the normalization process, it needs to be able to query the font about other characters as well.)

The fix is simply to make the HBGetGlyph callback return FALSE if the character was mapped to the .notdef glyph; this will suppress the unwanted compositions.
Assignee: nobody → jfkthame
Attachment #625602 - Flags: review?(smontagu)
(Reporter)

Updated

5 years ago
Attachment #625602 - Flags: review?(smontagu) → review+
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/95a2e9d06e31
OS: Linux → All
Hardware: x86 → All
Target Milestone: --- → mozilla15

Comment 4

5 years ago
https://hg.mozilla.org/mozilla-central/rev/95a2e9d06e31
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Duplicate of this bug: 761691
(Assignee)

Comment 6

5 years ago
This is currently on m-c and aurora; however, given that it's affecting real-world sites for some users (see bug 761691), and given the trivial nature of the patch, I suggest we should consider uplifting it to beta as well.
(Assignee)

Comment 7

5 years ago
Comment on attachment 625602 [details] [diff] [review]
patch, don't let harfbuzz apply [de]composition if the target chars aren't supported in the font

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 728866

User impact if declined: Broken display of certain Hebrew letter+diacritic combinations, depending on fonts used.

Testing completed (on m-c, etc.): Patch has been on m-c for 2+ weeks, and now on Aurora as well, with no adverse effects reported.

Risk to taking this patch (and alternatives if risky): Minimal risk - simple one-line change that is clearly correct. Alternative would be to switch off harfbuzz rendering for Hebrew script, but this would regress rendering of some more complex cases (multi-diacritic combinations).

String or UUID changes made by this patch: none
Attachment #625602 - Flags: approval-mozilla-beta?

Updated

5 years ago
Blocks: 762028
(Assignee)

Updated

5 years ago
Duplicate of this bug: 762028

Comment 9

5 years ago
Hello,

Thank you for fixing this.

If I understand correctly, this should get fixed in Firefox 14. Would it be possible to release a fix for Firefox 13 (it is affecting my personal site - see screenshots in bug 762028)?

Updated

5 years ago
status-firefox13: --- → affected
status-firefox14: --- → affected
tracking-firefox13: --- → +
tracking-firefox14: --- → +
Comment on attachment 625602 [details] [diff] [review]
patch, don't let harfbuzz apply [de]composition if the target chars aren't supported in the font

[Triage Comment]
Approved for Beta 14 since this is a recent regression in FF13, is low risk, and already has dupes within the first 24 hours after release.
Attachment #625602 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to ynovetsky from comment #9)
> If I understand correctly, this should get fixed in Firefox 14. Would it be
> possible to release a fix for Firefox 13 (it is affecting my personal site -
> see screenshots in bug 762028)?

We're now tracking this for possible inclusion in a 13.0.1 release.
(In reply to Jonathan Kew (:jfkthame) from comment #7)
> User impact if declined: Broken display of certain Hebrew letter+diacritic
> combinations, depending on fonts used.

One last thing - is there any way to reason about the number of sites affected?
Whiteboard: [qa+]

Comment 13

5 years ago
by'"`--

It would be a lot of sites for Israeli children and a lot of sites for adults that have any quotes from the Hebrew Bible (which is very common, of course) including Israeli forum sites on many subjects.

Some examples, http://www.mechon-mamre.org/ and http://kodesh.snunit.k12.il/ and http://www.mikranet.org.il/

The very name of the land and the state (yisra'el) is corrupted by this bug.
(Reporter)

Comment 14

5 years ago
Also sites that contain or quote poetry.  I originally saw the bug on Facebook statuses, but it is also all over sites like http://benyehuda.org, e.g. http://benyehuda.org/rachel/Rac005.html.

What makes the bug even more widespread than I realized earlier is that it affects the David font on Windows, which is probably one of the most commonly used Hebrew fonts.
(Assignee)

Comment 15

5 years ago
Transplanted to mozilla-beta for Fx14:
https://hg.mozilla.org/releases/mozilla-beta/rev/9bbe134693cf

If there's a 13.0.1 update, I think this could easily ride along, with no significant risk involved.
status-firefox14: affected → fixed

Comment 16

5 years ago
(In reply to Simon Montagu from comment #14)
> Also sites that contain or quote poetry. 
Also Yiddish in YIVO standard spelling. 
Yud + Hirik, Zwei yudn mit a pasekh and more...
Ex: http://i779.photobucket.com/albums/yy77/Kotlarsky/Varia/LastVersionFFvsMSIEinYiddish.jpg

Updated

5 years ago
Attachment #625602 - Flags: approval-mozilla-release+
(Assignee)

Comment 17

5 years ago
Pushed to mozilla-release for Firefox 13.0.1:
https://hg.mozilla.org/releases/mozilla-release/rev/a069b718abfd
status-firefox13: affected → fixed
(Reporter)

Updated

5 years ago
Duplicate of this bug: 762034
Mozilla/5.0 (X11; Linux i686; rv:14.0) Gecko/20100101 Firefox/14.0
Mozilla/5.0 (Windows NT 6.1; rv:14.0) Gecko/20100101 Firefox/14.0

Could initially reproduce the issue on 13.0 in Ubuntu 12.04 and Windows 7. Calling it verified for F14 in beta7 on the above mentioned platforms using the following site from Simon's comment 14: http://benyehuda.org/rachel/Rac005.html

No rectangles displayed in F14 beta7. To verify in F13 with the F13.0.1 candidate
status-firefox14: fixed → verified
Verified fixed in Firefox 13.0.1 build 1 candidate.
status-firefox13: fixed → verified

Comment 21

5 years ago
Simon, do we have reftests for harfbuzz fonts and the Hebrew alphabet? I know we have some Hebrew tests in the bidi section, but all I find with Harfbuzz are Indic scripts: http://mxr.mozilla.org/mozilla-central/source/layout/reftests/indic-shaping/reftest.list  Of course, I could be looking in the wrong place.  

Would it be possible to turn the test case you have on this bug into a series of reftests for Hebrew harfbuzz support?  Would that be worthwhile?
You need to log in before you can comment on or make changes to this bug.