Update Graphite2 library to 1.3.7

RESOLVED FIXED in Firefox 46

Status

()

Core
Graphics: Text
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: tsmith, Assigned: jfkthame)

Tracking

unspecified
mozilla48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 wontfix, firefox46+ fixed, firefox47+ fixed, firefox48 fixed, firefox-esr3846+ fixed, firefox-esr4546+ fixed)

Details

(Whiteboard: gfx-noted)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
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)
(Reporter)

Comment 1

2 years ago
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)
(Reporter)

Comment 2

2 years ago
Removed a few bugs that don't affect Firefox and should not impact the release.
No longer blocks: 1254497, 1254862, 1254883

Comment 3

2 years ago
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)
(Reporter)

Comment 4

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

Comment 5

2 years ago
The timelines aren't set yet. Probably a week or more, easily.

Comment 6

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

Updated

2 years ago
Blocks: 1255779
Whiteboard: gfx-noted
(Reporter)

Comment 7

2 years ago
Martin, could you please create 1.3.7? I ran tests over the weekend with no new issues.
Flags: needinfo?(martin_hosken)
(Reporter)

Updated

2 years ago
Blocks: 1255640
(Reporter)

Updated

2 years ago
Blocks: 1255559
(Reporter)

Updated

2 years ago
Blocks: 1255731

Comment 8

2 years ago
Released 1.3.7 available from usual locations, etc. (github and sourceforge).
Flags: needinfo?(martin_hosken)
(Assignee)

Comment 9

2 years ago
Created attachment 8730588 [details] [diff] [review]
Update graphite2 library to release 1.3.7

Here's the patch to update our copy. Who wants to review/rubber-stamp this for landing? Tyson?
Attachment #8730588 - Flags: review?(twsmith)
(Assignee)

Updated

2 years ago
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
(Reporter)

Comment 10

2 years ago
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)

Updated

2 years ago
Attachment #8730588 - Flags: review?(jd.bugzilla) → review+
Hi,

backed this out in  https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=7c210149e23c since one of the changes caused reftest failures like :

https://treeherder.mozilla.org/logviewer.html#?job_id=23888478&repo=mozilla-inbound
Flags: needinfo?(jfkthame)
(Assignee)

Comment 13

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

Comment 14

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

Comment 15

2 years ago
Created attachment 8731708 [details] [diff] [review]
Update graphite2 library to release 1.3.7
(Assignee)

Updated

2 years ago
Attachment #8730588 - Attachment is obsolete: true
(Assignee)

Comment 16

2 years ago
Comment on attachment 8731708 [details] [diff] [review]
Update graphite2 library to release 1.3.7

Carrying over r=jdaggett.
Attachment #8731708 - Flags: review+

Updated

2 years ago
Blocks: 1257639

Comment 18

2 years ago
Now that this is fixed in trunk, we should consider whether we want it on Aurora, Beta, and/or ESR branches.
status-firefox45: --- → wontfix
status-firefox46: --- → affected
status-firefox47: --- → affected
status-firefox48: --- → fixed
status-firefox-esr38: --- → affected
status-firefox-esr45: --- → affected
tracking-firefox46: --- → ?
tracking-firefox47: --- → ?
tracking-firefox-esr38: --- → ?
tracking-firefox-esr45: --- → ?
Flags: needinfo?(rkothari)
Flags: needinfo?(lhenry)

Comment 19

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e9558895348a
Status: ASSIGNED → RESOLVED
Last Resolved: 2 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)
(Assignee)

Comment 21

2 years ago
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+

Comment 23

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/d8cd4a36a442
status-firefox46: affected → fixed

Comment 24

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/a5ddf55a5f6c
status-firefox47: affected → fixed
Adding tracking belatedly
tracking-firefox46: ? → +
tracking-firefox47: ? → +
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.
tracking-firefox-esr45: ? → 46+
Flags: needinfo?(lhenry)
(Assignee)

Comment 27

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

Comment 29

2 years ago
This is obsoleted by the update to 1.3.8 that was just taken on both esr38 and esr45 in bug 1262846.
status-firefox-esr38: affected → fixed
status-firefox-esr45: affected → fixed
tracking-firefox-esr38: ? → 46+
Flags: needinfo?(sledru)
You need to log in before you can comment on or make changes to this bug.