Last Comment Bug 747834 - Script codes broken by harfbuzz update
: Script codes broken by harfbuzz update
: regression
Product: Core
Classification: Components
Component: Graphics: Text (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: Jonathan Kew (:jfkthame)
: Milan Sreckovic [:milan]
Depends on:
Blocks: 738101 745780
  Show dependency treegraph
Reported: 2012-04-22 23:28 PDT by Simon Montagu :smontagu
Modified: 2012-04-25 07:15 PDT (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

rerun (119.77 KB, patch)
2012-04-22 23:30 PDT, Simon Montagu :smontagu
no flags Details | Diff | Splinter Review
Previous patch updated (120.38 KB, patch)
2012-04-22 23:33 PDT, Simon Montagu :smontagu
no flags Details | Diff | Splinter Review
patch, avoid dependency on script code ordering in script-run itemizer (1.60 KB, patch)
2012-04-23 01:39 PDT, Jonathan Kew (:jfkthame)
no flags Details | Diff | Splinter Review
patch, make genUnicodePropertyData tool independent of the listing of script codes in harfbuzz (3.93 KB, patch)
2012-04-23 02:46 PDT, Jonathan Kew (:jfkthame)
smontagu: review+
Details | Diff | Splinter Review

Description Simon Montagu :smontagu 2012-04-22 23:28:12 PDT
Regenerating nsUnicodeScriptCodes.h with after the harfbuzz update in 745780 causes lots of failures, as can be seen at

That push included patches for bug 738101 and other stuff, but the build failures on Linux and reftest oranges on other platforms are all reproducible without any of the changes from there, just by re-running on a clean tree.
Comment 1 Simon Montagu :smontagu 2012-04-22 23:30:56 PDT
Created attachment 617396 [details] [diff] [review]

Also, the #include of hb-common.h needs to be changed to hb.h
Comment 2 Simon Montagu :smontagu 2012-04-22 23:33:59 PDT
Created attachment 617398 [details] [diff] [review]
Previous patch updated
Comment 3 Jonathan Kew (:jfkthame) 2012-04-23 01:27:43 PDT
In the recent harfbuzz update, Behdad decided to rearrange the list that defines his HB_SCRIPT_* codes into "script age" and then alphabetical order (whereas previously it matched the script enums found in pango and glib). The rearrangement doesn't affect HB itself, as it's using tags rather than a sequentially-allocated list of codes. However, it means that the genUnicodePropertyData tool finds the codes in a different order, and hence generates different enum values.

So this causes a couple of issues for us. First, gfxPangoFonts assumes that MOZ_SCRIPT_* codes are equal to PANGO_SCRIPT_* codes, and can just be cast from one to the other when we want to call into pango. The static assertions in gfxPangoFonts.cpp were included in order to alert us if this assumption were to break.

And second, there are a couple of places in gfxScriptItemizer.cpp where we assume that COMMON and INHERITED are the first two codes, as we check script<=INHERITED to see whether a "real" script code has been encountered. This could easily be modified to explicitly check the codes without assuming anything about their ordering. Apart from this, I don't think our code makes any assumptions about the specific values of the MOZ_SCRIPT_* constants.

As for the mismatch with pango, I think we have a couple of options. Either we ensure that our script codes match pango's, by abandoning the use of hb-common.h as the source of the ordered list (maybe just hard-code the order in genUnicodePropertyData, and make the tool warn if it finds new HB_SCRIPT constants are defined that we don't know about yet?), or else we simply accept that our codes aren't expected to match pango's, and add a table that maps between them.
Comment 4 Jonathan Kew (:jfkthame) 2012-04-23 01:39:04 PDT
Created attachment 617412 [details] [diff] [review]
patch, avoid dependency on script code ordering in script-run itemizer

If we don't want to maintain a known ordering of the script codes (to match pango and glib), then I believe this (untested) patch would fix the test failures on non-Linux platforms.

However, I'm feeling more inclined to say that we should keep the old script codes, and abandon the dependency on how they happen to be listed in hb-common. ISTM that either way, we're going to have to bite the bullet and maintain a list somewhere - either the order of constants we want to use, or the mapping between our arbitrarily-generated ordering and pango's externally-defined one.
Comment 5 Jonathan Kew (:jfkthame) 2012-04-23 02:46:10 PDT
Created attachment 617432 [details] [diff] [review]
patch, make genUnicodePropertyData tool independent of the listing of script codes in harfbuzz

This should avoid the fragile dependency on the harfbuzz header. It still checks that all the script codes defined by harfbuzz are also defined for MOZ_SCRIPT_*, but the constants are no longer dependent on the order of the HB list.
Comment 6 Jonathan Kew (:jfkthame) 2012-04-23 02:47:28 PDT
(Note that if we do this, we don't need the patch to gfxScriptRunItemizer.)

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