Closed Bug 1255158 Opened 4 years ago Closed 4 years ago

Update Graphite2 library to 1.3.7

Categories

(Core :: Graphics: Text, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox45 --- wontfix
firefox46 + fixed
firefox47 + fixed
firefox48 --- fixed
firefox-esr38 46+ fixed
firefox-esr45 46+ fixed

People

(Reporter: tsmith, Assigned: jfkthame)

References

Details

(Whiteboard: gfx-noted)

Attachments

(1 file, 1 obsolete file)

Let's start the process of moving to graphite 1.3.7

Martin: we will need you to create a tag in the graphite repository for the release. Please include bug 1255055 and bug 1254925 (the latest bugs)
I should make that a little more clear...

Martin: Please create the tag to include fixes for bug 1255055 and bug 1254925, so hold of on the 1.3.7 release until those bugs are fixed if possible. If you have other plans in terms of release timeline please let us know.
Flags: needinfo?(martin_hosken)
Removed a few bugs that don't affect Firefox and should not impact the release.
No longer blocks: 1254497, 1254862, 1254883
It is anticipated that this 1.3.7 will be back ported to earlier versions of firefox? I can do a 1.3.7 release in the next day or so. All dependent bugs are fixed, I think. Let's have one more night to let the dust settle a little. Or is this super urgent?
Flags: needinfo?(martin_hosken)
(In reply to martin_hosken from comment #3)
> It is anticipated that this 1.3.7 will be back ported to earlier versions of
> firefox? I can do a 1.3.7 release in the next day or so. 
Yes this is in anticipation of the next release 1.3.7 or whatever the proper version will be.

> All dependent bugs are fixed, I think. Let's have one more night to let the dust settle a
> little. Or is this super urgent?
Al: What kind of time lines are we looking at? When do we need the next graphite release?
Blocks: 1255505
The timelines aren't set yet. Probably a week or more, easily.
Releasing graphite takes < 1 day and < 1hr if in a hurry. I would prefer to get as many bug fixes into the release as I can, so I'll aim for a comfortable T-2 days if that works for you guys.
Blocks: 1255779
Martin, could you please create 1.3.7? I ran tests over the weekend with no new issues.
Flags: needinfo?(martin_hosken)
Blocks: 1255640
Blocks: 1255559
Blocks: 1255731
Released 1.3.7 available from usual locations, etc. (github and sourceforge).
Flags: needinfo?(martin_hosken)
Here's the patch to update our copy. Who wants to review/rubber-stamp this for landing? Tyson?
Attachment #8730588 - Flags: review?(twsmith)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Comment on attachment 8730588 [details] [diff] [review]
Update graphite2 library to release 1.3.7

Review of attachment 8730588 [details] [diff] [review]:
-----------------------------------------------------------------

I think John is the best person for the review.
Attachment #8730588 - Flags: review?(twsmith) → review?(jd.bugzilla)
Attachment #8730588 - Flags: review?(jd.bugzilla) → review+
The failures here happen because the new graphite version rejects the (old, experimental) PigLatin font that we're using in a couple of tests. Most likely the font is indeed invalid, and the stricter checking in the new engine doesn't like it.
Flags: needinfo?(jfkthame)
(In reply to Jonathan Kew (:jfkthame) from comment #13)
> The failures here happen because the new graphite version rejects the (old,
> experimental) PigLatin font that we're using in a couple of tests. Most
> likely the font is indeed invalid, and the stricter checking in the new
> engine doesn't like it.

Turns out this was actually a bug in the engine, fixed in 7dc29f3e4665b17f6e825957b707db6da36ae7d8, so I've added that (3-line) fix to our patch here.
Attachment #8730588 - Attachment is obsolete: true
Comment on attachment 8731708 [details] [diff] [review]
Update graphite2 library to release 1.3.7

Carrying over r=jdaggett.
Attachment #8731708 - Flags: review+
Blocks: 1257639
Now that this is fixed in trunk, we should consider whether we want it on Aurora, Beta, and/or ESR branches.
Flags: needinfo?(rkothari)
Flags: needinfo?(lhenry)
https://hg.mozilla.org/mozilla-central/rev/e9558895348a
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
I think it makes sense to take it in 47 and 46. Jonathan, could you please nominate the patch for uplift to Aurora and Beta? For esr45 and esr38, I believe the decision was to keep it disabled.
Flags: needinfo?(rkothari) → needinfo?(jfkthame)
Comment on attachment 8731708 [details] [diff] [review]
Update graphite2 library to release 1.3.7

Approval Request Comment
[Feature/regressing bug #]: graphite2

[User impact if declined]: potential vulnerabilities

[Describe test coverage new/current, TreeHerder]: existing reftests for basic graphite functionality

[Risks and why]: minimal - no functional changes, just fixes for a bunch of potential vulnerabilities (overflows, etc)

[String/UUID change made/needed]: none


Note that graphite is currently preffed-off on beta; I assume we'd leave that as-is for the current cycle, at least, until things appear more stable. Nevertheless, I think uplifting this to beta makes sense: it means those users who do enable it will get the benefit of the fixes, and given that it's off by default, the uplift carries no real risk for everyone else.
Flags: needinfo?(jfkthame)
Attachment #8731708 - Flags: approval-mozilla-beta?
Attachment #8731708 - Flags: approval-mozilla-aurora?
Comment on attachment 8731708 [details] [diff] [review]
Update graphite2 library to release 1.3.7

We should fix these vulnerabilities even if graphite2 is disabled by default on beta. Uplifting for beta 4 and for aurora.
Flags: needinfo?(lhenry)
Attachment #8731708 - Flags: approval-mozilla-beta?
Attachment #8731708 - Flags: approval-mozilla-beta+
Attachment #8731708 - Flags: approval-mozilla-aurora?
Attachment #8731708 - Flags: approval-mozilla-aurora+
Adding tracking belatedly
Are we going to leave graphite turned off forever on ESR 45? Will put users in markets that need/want these fonts at risk, especially if they're Linux users and get an ESR-based distro. We should land this on ESR-45. I think we can leave graphite disabled on ESR-38 given we're at the last release.
Flags: needinfo?(lhenry)
In bug 1262846, we've updated to graphite 1.3.8 (so that supersedes this version), and I have requested esr45 approval for that (even though it's currently disabled); I'd suggest that's where we should make this decision.

I'd agree that leaving things unchanged on esr38 seems fine.
Sylvestre, over to you for esr45
Flags: needinfo?(lhenry) → needinfo?(sledru)
This is obsoleted by the update to 1.3.8 that was just taken on both esr38 and esr45 in bug 1262846.
Flags: needinfo?(sledru)
You need to log in before you can comment on or make changes to this bug.