833283 - remove the option to bypass OTS for downloaded fonts

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Jonathan Kew [:jfkthame]]

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.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: remove the option to bypass OTS for downloaded fonts

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: remove the option to bypass OTS for downloaded fonts

Minor correction - we mustn't discard the original WOFF data until we've copied the metadata block, if any.

Comment 3

[John Daggett (:jtd)]

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.

Comment 4

[Jonathan Kew [:jfkthame]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/4da2cd42a969

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4da2cd42a969

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/32a44be4703c

Comment 7

[John Daggett (:jtd)]

(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?

Comment 8

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

(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.

Comment 9

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

(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

[Behdad Esfahbod]

Not having the option is, rather inconvenient.  Would you reconsider this change?

Comment 11

[Jonathan Kew [:jfkthame]]

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

[Behdad Esfahbod]

(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

Comment 13

[Jonathan Kew [:jfkthame]]

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

[Behdad Esfahbod]

Sounds good.  Will do.  Thanks.