remove the option to bypass OTS for downloaded fonts

RESOLVED FIXED in mozilla21

Status

()

defect
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: jfkthame, Assigned: jfkthame)

Tracking

unspecified
mozilla21
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

7 years ago
We landed the OTS font sanitizer for gecko-1.9.2 (and then backported to 1.9.1). It's been switched on by default ever since; although there's a pref to bypass it, we don't consider that a supported or secure option. At this point, I think we can consider it stable/tested/trusted enough that there's no longer any need for a preference to disable it; we can make it a non-negotiable part of downloaded font support. Eliminating that pref also allows us to remove the old (and *much* less thorough) font validation we used to use, and the separate WOFF support code, as OTS handles WOFF decoding internally.
Assignee

Comment 1

7 years ago
Attachment #704810 - Flags: review?(jdaggett)
Assignee

Comment 2

7 years ago
Minor correction - we mustn't discard the original WOFF data until we've copied the metadata block, if any.
Attachment #704829 - Flags: review?(jdaggett)
Assignee

Updated

7 years ago
Attachment #704810 - Attachment is obsolete: true
Attachment #704810 - Flags: review?(jdaggett)
Comment on attachment 704829 [details] [diff] [review]
remove the option to bypass OTS for downloaded fonts

Looks good.  I think we could still probably reduce the overall number of times the font data gets copied around but that's easy enough to deal with later.
Attachment #704829 - Flags: review?(jdaggett) → review+
Assignee

Comment 4

7 years ago
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/4da2cd42a969
Target Milestone: --- → mozilla21
https://hg.mozilla.org/mozilla-central/rev/4da2cd42a969
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Depends on: 835473
(In reply to Ryan VanderMeulen [:RyanVM][UTC-4] from comment #6)
> https://hg.mozilla.org/mozilla-central/rev/32a44be4703c

Ryan, not sure why you're connecting that changeset to this bug.  Whoops?
(In reply to John Daggett (:jtd) from comment #7)
> (In reply to Ryan VanderMeulen [:RyanVM][UTC-4] from comment #6)
> > https://hg.mozilla.org/mozilla-central/rev/32a44be4703c
> 
> Ryan, not sure why you're connecting that changeset to this bug.  Whoops?

It was in bug 833382, but had a typo in the bug number in the changeset pointing to this bug instead.
(And note that Ryan is presumably just using https://mcmerge.mozilla.org/ to put the data from merges into Bugzilla; though m-cMerge allows manual review, the person doing the merge can't catch everything, and it's really just the fault of the data in the changesets he's merging.)

Comment 10

5 years ago
Not having the option is, rather inconvenient.  Would you reconsider this change?
Assignee

Comment 11

5 years ago
Hmm. Care to be more explicit about the inconvenience?

I don't think we'd want to directly revert the change here (which allowed us to drop a whole bunch of code that we don't want to keep maintaining), but we might be able to solve it in a simpler way via the new OTS TableAction callback.

Comment 12

5 years ago
(In reply to Jonathan Kew (:jfkthame) from comment #11)
> Hmm. Care to be more explicit about the inconvenience?
> 
> I don't think we'd want to directly revert the change here (which allowed us
> to drop a whole bunch of code that we don't want to keep maintaining), but
> we might be able to solve it in a simpler way via the new OTS TableAction
> callback.

I've been working on handcrafting a font for certain HTML uses. [1].  I've hit many OTS issues, which I've reported and fixed.  However, without a preference for circumventing OTS, I have no way of testing my font short of recompiling Firefox.  I understand that this is also true with any other part of Firefox.  Still would be very convenient to either be able to disable OTS, or make Firefox use my custom OTS.  I understand that neither option may be very interesting from a maintenance point of view for you.  Still wanted to raise the need.

[1] https://groups.google.com/forum/#!topic/fonttools/JR1yvwyHj-I
Assignee

Comment 13

5 years ago
Well, we could trivially support a pref that makes the TableAction callback return PASSTHRU for all tables.

However, I just tried this, and it doesn't work, because OTS no longer notices what tables are present at all, and rejects all fonts as having neither TT nor PS outlines. (And would presumably reject for several other reasons as well.)

If you/Khaled can tweak OTS such that using PASSTHRU for all standard tables works as expected (i.e. the top-level OTS code still knows what tables were present, so it can accept the font as a whole) then I think we could give you a solution.

(It'd be best to file a new bug for the Gecko side of this, though, rather than continuing here in an old resolved:fixed bug.)

Comment 14

5 years ago
Sounds good.  Will do.  Thanks.
You need to log in before you can comment on or make changes to this bug.