Closed Bug 1222124 Opened 9 years ago Closed 9 years ago

MOZ_ANDROID_EXCLUDE_FONTS should not include mathml*.properties files

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(firefox45 fixed)

RESOLVED FIXED
Firefox 45
Tracking Status
firefox45 --- fixed

People

(Reporter: sebastian, Assigned: gbrown)

References

Details

Attachments

(1 file)

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.
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)
(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.
(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!
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: