769194 - src:local(...) in @font-face rules doesn't work on Android

Status: RESOLVED FIXED

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

Description

[Jonathan Kew [:jfkthame]]

There's code in gfxFT2FontList that is intended to handle src:local(...) descriptors (by providing an implementation of LookupLocalFont), but this feature is currently broken in multiple ways.

First, gfxAndroidPlatform doesn't actually connect the LookupLocalFont method to the font list, so it's a no-op at the platform level; and second, the code in gfxFT2FontList is broken such that (a) it will often fail to find the intended font because it doesn't ensure case-insensitivity, and (b) if it finds the target family, it's likely to crash while iterating over the faces if any of the four "standard" faces (R/I/B/BI) were not present, resulting in NULL entries in the family's font list.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: patch, unbreak and hook up the src:local support for Android

This seems to fix the src:local issues in my initial testing, at least. Pushed to tryserver to check for general breakage...
https://tbpl.mozilla.org/?tree=Try&rev=8b96b8574883

Comment 2

[Jonathan Kew [:jfkthame]]

Pushed to inbound, along with removing the fails-if(Android) annotation from the font-face/local-1 and font-face/local-styled-1 reftests, as they're now expected to pass on all platforms.

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

Comment 3

[Ryan VanderMeulen [:RyanVM]]

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

Comment 4

[Jonathan Kew [:jfkthame]]

Backed out because this seems to be causing crashiness, see bug 770263.

https://hg.mozilla.org/integration/mozilla-inbound/rev/023ac6edd2af

Comment 5

[Ryan VanderMeulen [:RyanVM]]

(In reply to Jonathan Kew (:jfkthame) from comment #4)
> Backed out because this seems to be causing crashiness, see bug 770263.
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/023ac6edd2af

https://hg.mozilla.org/mozilla-central/rev/023ac6edd2af

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: support src:local(...) in @font-face rules on Android/FT2FontList.

Updated patch to apply to latest trunk. The crashiness last time this landed (bug 770263) was because we failed to copy the filename and index fields when cloning the font entry in LookupLocalFont; with that fixed, I can no longer reproduce the crashes.

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: update reftests for @font-face src:local, now working on Android and B2G.

Update the src:local reftests to include appropriate font names for B2G as well.

Comment 8

[Jonathan Kew [:jfkthame]]

Checking that the src:local reftests now pass on both Android and B2G:
https://tbpl.mozilla.org/?tree=Try&rev=c82a8b058075

Comment 9

[John Daggett (:jtd)]

Comment on attachment 8401727 [details] [diff] [review]
support src:local(...) in @font-face rules on Android/FT2FontList.

Looks good!

Comment 10

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7deee7bd215d
https://hg.mozilla.org/integration/mozilla-inbound/rev/a09193c70bc8

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7deee7bd215d
https://hg.mozilla.org/mozilla-central/rev/a09193c70bc8