Last Comment Bug 745780 - update harfbuzz to latest code from upstream
: update harfbuzz to latest code from upstream
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics: Text (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla14
Assigned To: Jonathan Kew (:jfkthame)
:
: Milan Sreckovic [:milan]
Mentors:
Depends on: 695857 747834
Blocks: 676068 758236
  Show dependency treegraph
 
Reported: 2012-04-16 09:30 PDT by Jonathan Kew (:jfkthame)
Modified: 2016-04-07 18:19 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch, harfbuzz code from upstream (166.77 KB, patch)
2012-04-16 11:45 PDT, Jonathan Kew (:jfkthame)
jd.bugzilla: review+
mark.finkle: approval‑mozilla‑central+
Details | Diff | Splinter Review
reftests for arabic presentation-form shaping (86.17 KB, patch)
2012-04-16 11:48 PDT, Jonathan Kew (:jfkthame)
jd.bugzilla: review+
Details | Diff | Splinter Review
patch from bug 727736 rebased on top of the harfbuzz update (4.54 KB, patch)
2012-04-20 00:31 PDT, Jonathan Kew (:jfkthame)
jd.bugzilla: review+
mark.finkle: approval‑mozilla‑central+
Details | Diff | Splinter Review

Description User image Jonathan Kew (:jfkthame) 2012-04-16 09:30:36 PDT
It'd be good to take a harfbuzz update now that Arabic fallback (presentation-form) shaping is implemented; this will be particularly beneficial on Android devices that are shipping with OpenType-less versions of Droid Sans Arabic.
Comment 1 User image Jonathan Kew (:jfkthame) 2012-04-16 11:45:31 PDT
Created attachment 615402 [details] [diff] [review]
patch, harfbuzz code from upstream
Comment 2 User image Jonathan Kew (:jfkthame) 2012-04-16 11:48:41 PDT
Created attachment 615403 [details] [diff] [review]
reftests for arabic presentation-form shaping

Note that the testcase involving a diacritic within the lam-alef ligature will currently fail (marked accordingly in the manifest), as Behdad hasn't yet implemented support for that. I expect that'll come in a minor update before long, but would like to get the basic code landed ASAP so that we can get it into the Android product; the use of diacritics within ligatures is a rare enough edge case that it's not critical if we don't have that right away.
Comment 3 User image John Daggett (:jtd) 2012-04-17 22:22:09 PDT
Comment on attachment 615403 [details] [diff] [review]
reftests for arabic presentation-form shaping

> +fails HTTP(..) != arabic-fallback-4.html arabic-fallback-4-notref.html

Why the failure here?
Comment 4 User image John Daggett (:jtd) 2012-04-17 22:24:20 PDT
(In reply to John Daggett (:jtd) from comment #3)

> Why the failure here?

Ah that's the lam-alef ligature.  Let's comment that out for now with a comment noting what it's testing.
Comment 5 User image John Daggett (:jtd) 2012-04-19 23:56:48 PDT
I'm seeing a couple reftest failures on try under XP:
https://tbpl.mozilla.org/php/getParsedLog.php?id=11061026&tree=Try&full=1

bidi/bidi-006.html
bidi/bidi-006-j.html

The usual suspects...
Comment 6 User image Jonathan Kew (:jfkthame) 2012-04-20 00:30:06 PDT
Oh yes - that's bug 727736 again. As that isn't yet addressed in upstream harfbuzz, we need to re-apply our patch locally. I'll attach it here for reference.
Comment 7 User image Jonathan Kew (:jfkthame) 2012-04-20 00:31:00 PDT
Created attachment 616894 [details] [diff] [review]
patch from bug 727736 rebased on top of the harfbuzz update
Comment 8 User image John Daggett (:jtd) 2012-04-20 00:33:24 PDT
Comment on attachment 615403 [details] [diff] [review]
reftests for arabic presentation-form shaping

r+ with failing test commented out for now
Comment 9 User image John Daggett (:jtd) 2012-04-20 00:35:18 PDT
Comment on attachment 615402 [details] [diff] [review]
patch, harfbuzz code from upstream

We're going to need reftests for the Indic shaping but we don't need those now since Indic shaping isn't on by default.
Comment 10 User image Jonathan Kew (:jfkthame) 2012-04-20 00:53:16 PDT
(In reply to John Daggett (:jtd) from comment #9)
> Comment on attachment 615402 [details] [diff] [review]
> patch, harfbuzz code from upstream
> 
> We're going to need reftests for the Indic shaping but we don't need those
> now since Indic shaping isn't on by default.

Right - it's known to be incomplete as yet (but Behdad has a test suite and is working on the failing issues).
Comment 11 User image Jonathan Kew (:jfkthame) 2012-04-20 01:08:25 PDT
Comment on attachment 615402 [details] [diff] [review]
patch, harfbuzz code from upstream

Requesting approval to land for mozilla-14. This *does* affect fennec, as we use harfbuzz for text shaping; the motivation for landing it now is that this update will fix the currently-completely-broken rendering of Arabic script on certain devices (e.g. LG Optimus, see bug 676068). Without this, the fennec product is unusable for Arabic-script readers with such devices.
Comment 12 User image Jonathan Kew (:jfkthame) 2012-04-20 01:10:27 PDT
Comment on attachment 616894 [details] [diff] [review]
patch from bug 727736 rebased on top of the harfbuzz update

This is just re-applying the patch from bug 727736 - needed along with the harfbuzz update so that we don't regress rendering of old Hebrew/Arabic fonts that lack proper diacritic support.

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