746975 - update graphite2 library to latest release

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[martin_hosken]

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]]

Attachment: patch, update graphite2 library to release 1.1.2

Comment 2

[Jonathan Kew [:jfkthame]]

Tryserver job is at https://tbpl.mozilla.org/?tree=Try&rev=0a7ee097be10.

Comment 3

[John Daggett (:jtd)]

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)]

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]

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)]

Heh, ok.

Comment 7

[Jonathan Kew [:jfkthame]]

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]]

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 9

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e8f573bf8f2e

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/e8f573bf8f2e