MOZ_ANDROID_EXCLUDE_FONTS should not include mathml*.properties files

RESOLVED FIXED in Firefox 45

Status

()

Firefox for Android
General
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: sebastian, Assigned: gbrown)

Tracking

unspecified
Firefox 45
All
Android
Points:
---

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(1 attachment)

MOZ_ANDROID_EXCLUDE_FONTS has been introduced in bug 1063868.

We are excluding everything in @BINPATH@/res/fonts/. This folder includes more than just the ttf files:

> -rw-rw-r-- 1 gbrown gbrown 92229 Sep  2 08:22 layout/mathml/mathfont.properties
> -rw-r--r-- 1 gbrown gbrown  7328 May  9  2014 layout/mathml/mathfontSTIXGeneral.properties
> -rw-r--r-- 1 gbrown gbrown  5017 Feb  7  2014 layout/mathml/mathfontUnicode.properties

Those files would get lost if we add support for downloadable fonts (bug 1194338) and start to use MOZ_ANDROID_EXCLUDE_FONTS in builds.

The files are quite small (~102KB). So we should rather ship them in the APK insteaf of adding them as downloadable content; especially given the fact that they currently are only loaded from omni.ja.
(In reply to Sebastian Kaspari (:sebastian) from comment #0)

> The files are quite small (~102KB). So we should rather ship them in the APK
> insteaf of adding them as downloadable content; especially given the fact
> that they currently are only loaded from omni.ja.

Yeah. This makes sense to me.
(Assignee)

Comment 2

2 years ago
Created attachment 8684018 [details] [diff] [review]
include mathfont properties even with MOZ_ANDROID_EXCLUDE_FONTS

I think this is all that's needed. I built with/without MOZ_ANDROID_EXCLUDE_FONTS and checked omni.ja: ttf + properties in normal apk; just mathfont properties when MOZ_ANDROID_EXCLUDE_FONTS=1.

I applied the same logic to b2gdroid -- I'm less sure about the implications there.
Attachment #8684018 - Flags: review?(s.kaspari)
Comment on attachment 8684018 [details] [diff] [review]
include mathfont properties even with MOZ_ANDROID_EXCLUDE_FONTS

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

Heh. I have written and tested exactly the same patch. :)
Attachment #8684018 - Flags: review?(s.kaspari) → review+
(In reply to Geoff Brown [:gbrown] from comment #2)
> I applied the same logic to b2gdroid -- I'm less sure about the implications
> there.

NI-ing fabrice. I assume that b2gdroid ships with fonts and has not MOZ_ANDROID_EXCLUDE_FONTS set.
Flags: needinfo?(fabrice)
Assignee: s.kaspari → gbrown
We ship moztt fonts in b2gdroid, and they end up in res/fonts (see http://mxr.mozilla.org/mozilla-central/source/build/Makefile.in#63). My understanding is that setting MOZ_ANDROID_EXCLUDE_FONTS will prevent them from being packaged because of http://mxr.mozilla.org/mozilla-central/source/mobile/android/b2gdroid/installer/package-manifest.in#564
That would not be good, so unless I'm mistaken, please don't enable this flag on b2gdroid.
Flags: needinfo?(fabrice)
(Assignee)

Comment 6

2 years ago
(In reply to [:fabrice] Fabrice Desré from comment #5)

Thanks Fabrice. I certainly won't be enabling MOZ_ANDROID_EXCLUDE_FONTS for b2gdroid. I will keep my patch as-is, including the b2gdroid change, just to keep the meaning of MOZ_ANDROID_EXCLUDE_FONTS the same between fennec and b2gdroid. That will allow us to use MOZ_ANDROID_EXCLUDE_FONTS on Android and won't affect b2gdroid.
(Assignee)

Comment 7

2 years ago
(In reply to Sebastian Kaspari (:sebastian) from comment #3)
> Heh. I have written and tested exactly the same patch. :)

Sorry to be so impatient. I really want to get this landed today!

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/80b56045dfb5
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
You need to log in before you can comment on or make changes to this bug.