Have a way to provide mobile-browser-specific resources

RESOLVED FIXED

Status

RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: alexp, Assigned: blassey)

Tracking

Trunk
All
Android
Dependency tree / graph

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

8 years ago
Some Android code (notifications, for example) requires images to be compiled into resources. We need a way to control that with easier customization.
(Reporter)

Comment 1

8 years ago
Created attachment 479907 [details] [diff] [review]
MOZ_ANDROID_RESOURCES_DIRECTORY variable

Added a new MOZ_ANDROID_RESOURCES_DIRECTORY variable to mobile confvars.sh.
It will be checked and used in the makefiles.
Assignee: nobody → alexp
Status: NEW → ASSIGNED
Attachment #479907 - Flags: review?(mark.finkle)
(Reporter)

Comment 2

8 years ago
Created attachment 479909 [details] [diff] [review]
Config changes for a new variable
Attachment #479909 - Flags: review?(ted.mielczarek)
(Reporter)

Updated

8 years ago
Blocks: 592088
(Reporter)

Updated

8 years ago
Attachment #479907 - Flags: review?(mark.finkle)
(Reporter)

Updated

8 years ago
Attachment #479909 - Flags: review?(ted.mielczarek)
(Reporter)

Comment 3

8 years ago
Created attachment 480287 [details] [diff] [review]
Config changes for a new variable

Alert service implementation for Android will be looking for MOZ_ANDROID_DRAWABLES variable, which could be provided in mobile/confvars.sh.
Attachment #479907 - Attachment is obsolete: true
Attachment #479909 - Attachment is obsolete: true
Attachment #480287 - Flags: review?(ted.mielczarek)
(Reporter)

Updated

8 years ago
Blocks: 601050
Why does this need to be a configure variable? From your usage in bug 601050, it looks like you could just do:
ifeq (Android,$(OS_TARGET))
ANDROID_DRAWABLES = ...
endif

in that Makefile. I don't see the need to put it in configure.
(Reporter)

Comment 5

8 years ago
(In reply to comment #4)
> I don't see the need to put it in configure.

The idea is to make this variable available in the platform code. Here's where it's checked and used:
http://mxr.mozilla.org/mozilla-central/source/embedding/android/Makefile.in#167

The variable simply defined in m-b/confvars.sh is not visible in that Makefile unless it's propagated through the configure.

Well, I might be missing something here, and would be glad to know if this could be done some other way.
That code is all getting heavily mangled in bug 567873 anyway.
(Reporter)

Comment 7

8 years ago
(In reply to comment #6)
> That code is all getting heavily mangled in bug 567873 anyway.

Yes, though I believe it's supposed to use the same approach for this bit. The app should be able somehow to define a list of drawable resources, which the packaging Makefile could pick up.
Hm. This is why we have packaging manifests, isn't it?

mwu: is there a better way to do this? This just feels wrong.

Comment 9

8 years ago
(In reply to comment #8)
> Hm. This is why we have packaging manifests, isn't it?
> 
> mwu: is there a better way to do this? This just feels wrong.

Sure. We can take the approach from https://bugzilla.mozilla.org/show_bug.cgi?id=526333 and add a platform specific manifest.
Yeah, let's do that.
(Reporter)

Updated

8 years ago
Depends on: 575751
Comment on attachment 480287 [details] [diff] [review]
Config changes for a new variable

Going to r- this for now, then.
Attachment #480287 - Flags: review?(ted.mielczarek) → review-
Created attachment 490228 [details] [diff] [review]
patch
Assignee: alexp → blassey.bugs
Attachment #480287 - Attachment is obsolete: true
Attachment #490228 - Flags: review?(mwu)
Created attachment 490229 [details] [diff] [review]
patch
Attachment #490229 - Flags: review?(mark.finkle)
Comment on attachment 490229 [details] [diff] [review]
patch

I haven't be thrilled by this approach. There is a weird, undocumented connection between the platform installer and the app installer.

I guess this will work until something better comes along.
Attachment #490229 - Flags: review?(mark.finkle) → review+

Comment 15

8 years ago
Comment on attachment 490228 [details] [diff] [review]
patch

This is terrible but I don't have time right now to make something better.
Attachment #490228 - Flags: review?(mwu) → review+
pushed http://hg.mozilla.org/mobile-browser/rev/4733aec2fa63 and http://hg.mozilla.org/mozilla-central/rev/f03f50294d9c
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.