Last Comment Bug 746975 - update graphite2 library to latest release
: update graphite2 library to latest release
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics: Text (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Jonathan Kew (:jfkthame)
:
:
Mentors:
Depends on: 721068
Blocks: 797863
  Show dependency treegraph
 
Reported: 2012-04-19 06:56 PDT by martin_hosken
Modified: 2015-08-30 23:36 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch, update graphite2 library to release 1.1.2 (64.11 KB, patch)
2012-04-19 07:30 PDT, Jonathan Kew (:jfkthame)
jd.bugzilla: review+
akeybl: approval‑mozilla‑central+
Details | Diff | Splinter Review

Description martin_hosken 2012-04-19 06:56:17 PDT
Due to a bug in the graphite2 engine, feature ids < 4 do not identify the right feature. This is fixed in the latest release of the graphite2 engine v1.1.2 which also has a few minor security bug fixes.

available from: http://sourceforge.net/projects/silgraphite/files/graphite2/graphite2-1.1.2.tgz/download or http://projects.palaso.org/attachments/download/227/graphite2-1.1.2.tgz
Comment 1 Jonathan Kew (:jfkthame) 2012-04-19 07:30:19 PDT
Created attachment 616564 [details] [diff] [review]
patch, update graphite2 library to release 1.1.2
Comment 2 Jonathan Kew (:jfkthame) 2012-04-19 08:58:19 PDT
Tryserver job is at https://tbpl.mozilla.org/?tree=Try&rev=0a7ee097be10.
Comment 3 John Daggett (:jtd) 2012-04-19 17:21:15 PDT
I think the title of the bug should be "update graphite to latest" or something like that.  Changes proposed for the 'font-feature-settings' syntax will mean that tags not four characters in length will be considered invalid.  See bug 718539, comment 34.

In standardizing how Graphite features are used on the web, I think we really should be encouraging developers to use existing OpenType tags where it makes sense.  For non-standard, or custom features, they can use the syntax for vendor-specific tags.  If it's really important to support a format specific to Graphite (or another *standardized* font technology) then adding syntax like 'graphite(feature-tag)' would make sense.  But right now I just don't see that as necessary, the existing pattern allows ample flexibility and we should encourage font developers to work with that pattern.
Comment 4 John Daggett (:jtd) 2012-04-19 20:13:29 PDT
Comment on attachment 616564 [details] [diff] [review]
patch, update graphite2 library to release 1.1.2

Looks fine.

-#define GR2_VERSION_MAJOR   2
-#define GR2_VERSION_MINOR   0
-#define GR2_VERSION_BUGFIX  1
+#define GR2_VERSION_MAJOR   1
+#define GR2_VERSION_MINOR   1
+#define GR2_VERSION_BUGFIX  2

Is this change right?  The version is *decreasing*?!?
Comment 5 martin_hosken 2012-04-19 20:27:45 PDT
Hmm. So later versions are worse than earlier ones. Ah well, just more evidence that the world is going to pot! In reality we just dropped the major version to sync with the public release number. Imagine an invisible epoch bump. It's a one off event. We are moving forward, honest :)

So, yes it is right.
Comment 6 John Daggett (:jtd) 2012-04-19 20:36:12 PDT
Heh, ok.
Comment 7 Jonathan Kew (:jfkthame) 2012-04-20 00:50:05 PDT
Comment on attachment 616564 [details] [diff] [review]
patch, update graphite2 library to release 1.1.2

Requesting approval to land for mozilla-14.

This presents negligible risk to the mobile beta product, as (a) it's a low-risk library update anyhow; but also (b) the feature is preffed off by default, and therefore the update has no impact whatsoever except for those users who deliberately enable it in about:config.

It's desirable to take the update for the sake of the graphite user community who explicitly switch on the feature - the new release fixes a bug affecting access to certain font features, and includes several minor security/robustness fixes.

The upstream release was deliberately timed to fit in with our published schedule for mozilla 14, so as to get the font-feature fix into that release. Given the absence of risk to mobile, I think it should be accepted as normal.
Comment 8 Alex Keybl [:akeybl] 2012-04-20 15:45:54 PDT
Comment on attachment 616564 [details] [diff] [review]
patch, update graphite2 library to release 1.1.2

(In reply to Jonathan Kew (:jfkthame) from comment #7)
> It's desirable to take the update for the sake of the graphite user
> community who explicitly switch on the feature - the new release fixes a bug
> affecting access to certain font features, and includes several minor
> security/robustness fixes.

Approved for central since this is pref'd off.
Comment 10 Phil Ringnalda (:philor) 2012-04-21 23:32:39 PDT
https://hg.mozilla.org/mozilla-central/rev/e8f573bf8f2e

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