update harfbuzz to latest code from upstream

RESOLVED FIXED in mozilla14

Status

()

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

People

(Reporter: jfkthame, Assigned: jfkthame)

Tracking

(Blocks: 1 bug)

unspecified
mozilla14
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Assignee)

Description

5 years ago
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.
(Assignee)

Updated

5 years ago
Blocks: 676068
(Assignee)

Comment 1

5 years ago
Created attachment 615402 [details] [diff] [review]
patch, harfbuzz code from upstream
Assignee: nobody → jfkthame
Attachment #615402 - Flags: review?(jdaggett)
(Assignee)

Comment 2

5 years ago
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.
Attachment #615403 - Flags: review?(jdaggett)

Comment 3

5 years ago
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

5 years ago
(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

5 years ago
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...
(Assignee)

Comment 6

5 years ago
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.
(Assignee)

Comment 7

5 years ago
Created attachment 616894 [details] [diff] [review]
patch from bug 727736 rebased on top of the harfbuzz update
Attachment #616894 - Flags: review?(jdaggett)

Comment 8

5 years ago
Comment on attachment 615403 [details] [diff] [review]
reftests for arabic presentation-form shaping

r+ with failing test commented out for now
Attachment #615403 - Flags: review?(jdaggett) → review+

Updated

5 years ago
Attachment #616894 - Flags: review?(jdaggett) → review+

Comment 9

5 years ago
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.
Attachment #615402 - Flags: review?(jdaggett) → review+
(Assignee)

Comment 10

5 years ago
(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).
(Assignee)

Comment 11

5 years ago
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.
Attachment #615402 - Flags: approval-mozilla-central?
(Assignee)

Comment 12

5 years ago
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.
Attachment #616894 - Flags: approval-mozilla-central?
Attachment #615402 - Flags: approval-mozilla-central? → approval-mozilla-central+
Attachment #616894 - Flags: approval-mozilla-central? → approval-mozilla-central+
(Assignee)

Comment 13

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/03fa311b5a9b (harfbuzz update)
https://hg.mozilla.org/integration/mozilla-inbound/rev/25ca02ba36f4 (re-apply bug 727736)
https://hg.mozilla.org/integration/mozilla-inbound/rev/62e48300103f (reftests)
Target Milestone: --- → mozilla14
https://hg.mozilla.org/mozilla-central/rev/03fa311b5a9b
https://hg.mozilla.org/mozilla-central/rev/25ca02ba36f4
https://hg.mozilla.org/mozilla-central/rev/62e48300103f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Depends on: 747834
Blocks: 758236
Depends on: 695857
You need to log in before you can comment on or make changes to this bug.