Last Comment Bug 756850 - Some combinations of Hebrew vowels and consonants not displayed in certain fonts with harfbuzz
: Some combinations of Hebrew vowels and consonants not displayed in certain fo...
Status: RESOLVED FIXED
[qa+]
: regression
Product: Core
Classification: Components
Component: Layout: Text (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla15
Assigned To: Jonathan Kew (:jfkthame)
:
:
Mentors:
: 761691 762028 762034 (view as bug list)
Depends on:
Blocks: 728866 762028
  Show dependency treegraph
 
Reported: 2012-05-20 02:00 PDT by Simon Montagu :smontagu
Modified: 2012-07-17 09:28 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
verified
+
verified


Attachments
Testcase (11.75 KB, text/html)
2012-05-20 02:00 PDT, Simon Montagu :smontagu
no flags Details
patch, don't let harfbuzz apply [de]composition if the target chars aren't supported in the font (938 bytes, patch)
2012-05-21 04:01 PDT, Jonathan Kew (:jfkthame)
smontagu: review+
akeybl: approval‑mozilla‑beta+
akeybl: approval‑mozilla‑release+
Details | Diff | Splinter Review

Description Simon Montagu :smontagu 2012-05-20 02:00:49 PDT
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.
Comment 1 Simon Montagu :smontagu 2012-05-20 03:04:46 PDT
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].
Comment 2 Jonathan Kew (:jfkthame) 2012-05-21 04:01:10 PDT
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.
Comment 4 Ed Morley [:emorley] 2012-05-22 05:02:32 PDT
https://hg.mozilla.org/mozilla-central/rev/95a2e9d06e31
Comment 5 Jonathan Kew (:jfkthame) 2012-06-06 00:36:54 PDT
*** Bug 761691 has been marked as a duplicate of this bug. ***
Comment 6 Jonathan Kew (:jfkthame) 2012-06-06 00:39:28 PDT
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.
Comment 7 Jonathan Kew (:jfkthame) 2012-06-06 00:44:55 PDT
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
Comment 8 Jonathan Kew (:jfkthame) 2012-06-06 08:42:17 PDT
*** Bug 762028 has been marked as a duplicate of this bug. ***
Comment 9 Inkbug 2012-06-06 09:44:49 PDT
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)?
Comment 10 Alex Keybl [:akeybl] 2012-06-06 16:07:02 PDT
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.
Comment 11 Alex Keybl [:akeybl] 2012-06-06 16:07:34 PDT
(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.
Comment 12 Alex Keybl [:akeybl] 2012-06-06 17:09:55 PDT
(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?
Comment 13 Shelomo 2012-06-06 18:44:46 PDT
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.
Comment 14 Simon Montagu :smontagu 2012-06-06 23:02:33 PDT
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.
Comment 15 Jonathan Kew (:jfkthame) 2012-06-07 02:02:26 PDT
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.
Comment 16 Shaul 2012-06-08 12:10:33 PDT
(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
Comment 17 Jonathan Kew (:jfkthame) 2012-06-13 07:50:55 PDT
Pushed to mozilla-release for Firefox 13.0.1:
https://hg.mozilla.org/releases/mozilla-release/rev/a069b718abfd
Comment 18 Simon Montagu :smontagu 2012-06-14 00:17:42 PDT
*** Bug 762034 has been marked as a duplicate of this bug. ***
Comment 19 Virgil Dicu [:virgil] [QA] 2012-06-14 08:12:17 PDT
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
Comment 20 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-06-14 15:37:56 PDT
Verified fixed in Firefox 13.0.1 build 1 candidate.
Comment 21 cmtalbert 2012-07-17 09:28:08 PDT
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?

Note You need to log in before you can comment on or make changes to this bug.