976334 - Provide ability to read config file from omni.ja

Status: UNCONFIRMED

(Core :: General, defect)

Description

[Nathan Toone [:toonetown]]

Attachment: read-config-from-omni.diff

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_1) AppleWebKit/537.73.11 (KHTML, like Gecko) Version/7.0.1 Safari/537.73.11

Steps to reproduce:

Currently loading a configuration file (via general.config.filename) *always* loads from the install directory.  It would be helpful in some cases (i.e. for branded installs) to be able to read the configuration file from the omni.ja that is built as well.



Expected results:

I propose that an option (general.config.is_bin_dir) is provided that can be set to false.  Doing so will then load the value of general.config.filename from resource://res/defaults/autoconfig/ instead of the install directory.  The code in nsReadConfig.cpp already supports this flag - it is just always a value of "true".

The default value should be false (current behavior)

A patch file is attached which does this

Comment 1

[Nathan Toone [:toonetown]]

I'd like to get a little bit of feedback on this, it should be fairly straightforward - but I'm wondering if there is something I'm missing - or if there is a better way to accomplish this.

The use case for this is that I have a few settings in a config file that I want to lock via lockPref.  Normally this config file needs to be loaded from the install directory, but for mobile, it would be a lot easier to read it from the omni.ja.  We can then include the config file in the omni.ja in a similar manner as suggested in bug 976357

Comment 2

[Nick Alexander :nalexander [he/him]]

I'm going to respond to both related issues here.

I'm confident that this approach will achieve what you want.  But I'm not sure it's the best way to achieve that, so help me understand.

The thing that I don't understand is how you will set general.config.is_bin_dir.  If you can set that early enough to impact the proceedings, can you not set autoadmin.global_config_url to something like "resource://android/assets/custom.cfg" or "resource://gre/custom.cfg" or whatever?  (I'm taking this pref out of https://mike.kaply.com/2012/03/16/customizing-firefox-autoconfig-files/, I really know nothing about autoconfig.)  So help me understand how you get started on this process.

A final thought: AUTOCFG_JS_EXPORTS is super special-case, and clearly not intended for multiple files.  Better to tuck a custom.cfg into assets/ than into the omni.ja.

Comment 3

[Nathan Toone [:toonetown]]

You set general.config.filename (and, with this patch, general.config.is_bin_dir) in the default prefs file. That triggers the "autoconfig" stuff. You can't set autoadmin.global_config_url from the prefs file - it has to be set from within the config file. 

That's the main purpose of this patch - to provide a way to put that config file *somewhere* as a part of the android package.  I just chose to do it within the omni.ja because it seemed like a "standard-ish" place to put it. 

Without this patch (or with general.config.is_bin_dir set to true...the default), then *only* the "install" directory is read.  Is there a way to build the Firefox for android package so that the needed config file is included in the "install" directory (wherever that is on android)?

Comment 4

[Nathan Toone [:toonetown]]

https://bugzilla.mozilla.org/show_bug.cgi?id=206294#c6 Mentions that the autoadmin.global_config_url has to be set in the config file and not the prefs file.

Comment 5

[Nathan Toone [:toonetown]]

And, actually, if https://bugzilla.mozilla.org/show_bug.cgi?id=440908 we're applied, then it would render my particular use case for this patch useless.  I might close this bug in favor of the other one (it has been around, with a patch, for 7 years - and has been explicitly denied multiple times, though...so maybe that's not the best approach)

Comment 6

[Mike Kaply [:mkaply]]

This seems wrong to be.

Why would an autoconfig end up as part of a omni.ja file? That's contrary to what the autoconfig is for...

I don't totally understand the usecase. Are you building your own Firefox?

Comment 7

[Nathan Toone [:toonetown]]

The main reason for this patch is so that certain preferences can be locked for a branded parental control browser based on firefox.  As I mentioned in comment 5, that's the exact reason why if bug 440908 were implemented, it would fulfill my use case.

Comment 8

[Mike Kaply [:mkaply]]

Why wouldn't you just bundle an autoconfig file with your branded parental control browser?

If you're worried about tampering, omni.js can be messed with as well.

But I do totally agree. bug 440908 is a good solution to this problem.

Comment 9

[Nick Alexander :nalexander [he/him]]

(In reply to Mike Kaply [:mkaply] from comment #8)
> Why wouldn't you just bundle an autoconfig file with your branded parental
> control browser?

I think the issue is that we don't know how to do this for Fennec, which doesn't really follow the GRE expectations.

Nathan, I expect that you want a file in the APK root, next to application.ini and package-name.txt.  It's not trivial to arrange a file to get there, but http://www.ncalexander.net/blog/2014/07/02/adding-assets-to-the-fennec-apk-file/ might help.  You can always hack up the packager around https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/upload-files.mk#472.

You could also add some logging to the C++ code, and potentially try reading the package-name.txt file at the relevant moment, just to see if my guess is correct.

Comment 10

[Mike Kaply [:mkaply]]

Totally missed the mobile thing. Sorry about that.

So where does NS_GRE_DIR point on mobile?

http://mxr.mozilla.org/mozilla-central/source/extensions/pref/autoconfig/src/nsReadConfig.cpp#238

I think what I'd rather see here is allowing the autoconfig file to have a path in it. That would be a nice solution.

Comment 11

[Nathan Toone [:toonetown]]

> I think what I'd rather see here is allowing the autoconfig file to have a
> path in it. That would be a nice solution.

I actually agree with this - I'd just like to see the autoconfig accept any path (is there a reason why it wasn't doing that anyway?)

Comment 12

[Nathan Toone [:toonetown]]

> So where does NS_GRE_DIR point on mobile?

On my application, it points to /data/users/0/com.myapp.identifier - which isn't a part of the APK at all...so if it's going to read from there, then it would need to be copied or something.  I don't know at what stage (in mobile, at least), we can copy that file and have it there early enough to be read by the autoconfig extension.