1241858 - Unofficial branded builds are no longer able to override devtools.js preferences

Status: RESOLVED WONTFIX

(Firefox Build System :: General, defect)

Description

[matthias koplenig [:metasieben]]

In version 44 unofficial branded builds are not able to override preferences set within devtools.js, afaics.

Comment 1

[matthias koplenig [:metasieben]]

I'm guessing the problem is within the loading mechanism for the default preferences.

currently these files are in /browser/defaults:
devtools.js
firefox-branding.js
firefox-l10n.js
firefox.js
webide-prefs.js

i'm wondering, if renaming devtools.js to firefox-devtools.js would solve the issue?

Comment 2

[matthias koplenig [:metasieben]]

Ryan, would it be possible to rename the preferences file to something other than devtools.js, since it seems to override preferences set in firefox-branding.js?

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to matthias koplenig from comment #2)
> Ryan, would it be possible to rename the preferences file to something other
> than devtools.js, since it seems to override preferences set in
> firefox-branding.js?

Can you help me understand the problem better?  What is an "unofficial" branded build?  It seems like the in-tree channel branding files apply as expected for the moment.

Is there something additional you are trying customize?

Comment 4

[matthias koplenig [:metasieben]]

firefox-branding.js from /browser/branding/unofficial/pref/ gets copied to /browser/defaults, since all other preferences-files in the directory are loaded before(afaik) firefox-branding.js, all prefs set there are overridden.

Since bug 1204808, devtools.js is loaded last(afaik), and every setting in firefox-branding.js is reset.

currently i'm changing some scratchpad-preferences(linenumbers/fontsize) and which panels are shown.

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

:Gijs, can you help me understand what's expected here?  How important is it that firefox-branding.js can be used to override any prefs?

Comment 6

[:Gijs (he/him)]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #5)
> :Gijs, can you help me understand what's expected here?  How important is it
> that firefox-branding.js can be used to override any prefs?

I don't think it normally needs to override any devtools prefs, unless we (a) want to specify a default in there that we then (b) want to override on a per-channel/branding-basis. Which seems unlikely. We currently specify the xss.count devtools pref only in the different branding files, so it's not specified in devtools.js or any of the Firefox files, so it doesn't run into this issue. But this could potentially be a "gotcha" if we wanted to add more of these prefs.

However, Matthias, it sounds like you're using the branding file to customize Firefox? Why not use the recommended options from https://developer.mozilla.org/en-US/Firefox/Enterprise_deployment ?

I'm also confused as to why this issue is restricted to "unofficial branding" - it seems to me like the other branding files are likely to have the same problem. Do they not?

(In reply to matthias koplenig from comment #4)
> firefox-branding.js from /browser/branding/unofficial/pref/ gets copied to
> /browser/defaults, since all other preferences-files in the directory are
> loaded before(afaik) firefox-branding.js, all prefs set there are overridden.

What specifies this order of loading? It's clearly not alphabetical...

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to :Gijs Kruitbosch from comment #6)
> (In reply to matthias koplenig from comment #4)
> > firefox-branding.js from /browser/branding/unofficial/pref/ gets copied to
> > /browser/defaults, since all other preferences-files in the directory are
> > loaded before(afaik) firefox-branding.js, all prefs set there are overridden.
> 
> What specifies this order of loading? It's clearly not alphabetical...

When I last investigated a similar issue in bug 1174234, I learned that we seem to load them in reverse alphabetical order[1]. (Why...???)

So, that would suggest they are processed in the order:

webide.js
firefox.js
firefox-l10n.js
firefox-branding.js
devtools.js

[1]: https://dxr.mozilla.org/mozilla-central/rev/eb25b90a05c194bfd4f498ff3ffee7440f85f1cd/modules/libpref/Preferences.cpp#1039-1041

Comment 8

[matthias koplenig [:metasieben]]

I'm building my own fork of Firefox, mostly to use a different default profile(-directory) (and name/icon). Basically I used /browser/branding/unofficial as a template and created my own branding-files.

All other branding-options(nightly/aurora/official) should show the same results. 

Documentation on branded builds is pretty much non-existent, but since it worked without any problems before 1204808 landed, I assumed that firefox-branding.js should be able to override all other preferences set in /browser/defaults.

Looks like this was an unintended side-effect of the loadorder: firefox-branding.js overriding firefox.js


afaik we are at least overriding one pref('browser.search.geoSpecificDefaults') in firefox-l10n.js, which is set in firefox.js

Comment 9

[:Gijs (he/him)]

Seems like it make a slight (but actual) difference for branding if we renamed devtools.js to firefox-devtools.js - Ryan, does this answer your question?

Comment 10

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to :Gijs Kruitbosch from comment #9)
> Seems like it make a slight (but actual) difference for branding if we
> renamed devtools.js to firefox-devtools.js - Ryan, does this answer your
> question?

Yes, I suppose so...  Although if firefox-branding.js is truly meant to be the last default prefs file, it seems like it should be named "000-firefox-branding.js" or something so it's always run last (by sorting first alphabetically).

I guess what I mean is "firefox-branding.js" is the one we want to have a specific ordering here, so that it's applied last.  It seems like it was mostly just chance that allowed it to be the last file applied historically, since we haven't taken any particular steps (AFAICT) to run the files in a particular order in the past, and other future prefs files other than devtools.js could cause this same problem again by applying after the branding one.

Could we instead rename firefox-branding.js such that it's always the last file to be read?  Or does that break things?

Comment 11

[Mike Kaply [:mkaply]]

Couldn't we take advantage of the "special filenames feature" and force firefox-branding.hs to be the last file read?

Comment 12

[:Gijs (he/him)]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #10)
> (In reply to :Gijs Kruitbosch from comment #9)
> > Seems like it make a slight (but actual) difference for branding if we
> > renamed devtools.js to firefox-devtools.js - Ryan, does this answer your
> > question?
> 
> Yes, I suppose so...  Although if firefox-branding.js is truly meant to be
> the last default prefs file, it seems like it should be named
> "000-firefox-branding.js" or something so it's always run last (by sorting
> first alphabetically).
> 
> I guess what I mean is "firefox-branding.js" is the one we want to have a
> specific ordering here, so that it's applied last.  It seems like it was
> mostly just chance that allowed it to be the last file applied historically,
> since we haven't taken any particular steps (AFAICT) to run the files in a
> particular order in the past, and other future prefs files other than
> devtools.js could cause this same problem again by applying after the
> branding one.
> 
> Could we instead rename firefox-branding.js such that it's always the last
> file to be read?  Or does that break things?

I'm not sure about the build/distribution system part of this, needinfo'ing glandium and mkaply about that.

FWIW, based on the API in https://dxr.mozilla.org/mozilla-central/rev/eb25b90a05c194bfd4f498ff3ffee7440f85f1cd/modules/libpref/Preferences.cpp#1039-1041 it also looks like we could just add firefox-branding.js as the first element in 

https://dxr.mozilla.org/mozilla-central/rev/2b5237c178ea02133a777396c24dd2b713f2b8ee/modules/libpref/Preferences.cpp#1267-1281 , maybe?

Comment 13

[:Gijs (he/him)]

Hrm, except that it seems that code loads prefs from like 5 different places and I don't actually know off-hand which of them has firefox-branding.js :-\

Comment 14

[Mike Kaply [:mkaply]]

This shouldn't affect distribution. We do our own custom loading of things from distribution.ini.

As a side note, this has come up in enterprise before and is the reason I name my autoconfig file autoconfig.js in the defaults/pref directory. (Although I'm switching to aautoconfig.js to load after the old all-settings.js).

Comment 15

[Mike Hommey [:glandium]]

If one wants to change the defaults in their fork of Firefox, they have plenty of ways to do it that doesn't involve using firefox-branding.js. Heck, they could even add a pref file in the directory, instead of using an existing file.

Comment 16

[:Gijs (he/him)]

(In reply to Mike Hommey [:glandium] from comment #15)
> If one wants to change the defaults in their fork of Firefox, they have
> plenty of ways to do it that doesn't involve using firefox-branding.js.
> Heck, they could even add a pref file in the directory, instead of using an
> existing file.

I think there are 2 viable options here.

1) wontfix.

2) for our own, Firefox-internal purposes, we should be able to override devedition prefs from firefox-branding.js, in which case we should still fix this issue somehow.

I'm happy either way, but we should make a decision. I do think that if we decide that we should be able to use firefox-branding.js for prefs in devtools.js as we do for prefs in firefox.js or all.js, then we should fix this bug, and that would be a valid concern to have and a valid reason to fix it. I just don't know if it's really conceivable and/or worth putting effort if/until we actually have such prefs which we need to override.

Delegating the decision about this back to Ryan. :-)

Comment 17

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to :Gijs Kruitbosch from comment #16)
> (In reply to Mike Hommey [:glandium] from comment #15)
> > If one wants to change the defaults in their fork of Firefox, they have
> > plenty of ways to do it that doesn't involve using firefox-branding.js.
> > Heck, they could even add a pref file in the directory, instead of using an
> > existing file.
> 
> I think there are 2 viable options here.
> 
> 1) wontfix.
> 
> 2) for our own, Firefox-internal purposes, we should be able to override
> devedition prefs from firefox-branding.js, in which case we should still fix
> this issue somehow.
> 
> I'm happy either way, but we should make a decision. I do think that if we
> decide that we should be able to use firefox-branding.js for prefs in
> devtools.js as we do for prefs in firefox.js or all.js, then we should fix
> this bug, and that would be a valid concern to have and a valid reason to
> fix it. I just don't know if it's really conceivable and/or worth putting
> effort if/until we actually have such prefs which we need to override.
> 
> Delegating the decision about this back to Ryan. :-)

I think, at least at this moment in time, we don't have a need for for firefox-branding.js to override prefs in devtools.js, and I agree it's probably best to wait until we have such a case before making a change here.  So, I'll mark this as WONTFIX.  If the situation changes, we can revisit and make a change in the future.

It sounds like the reporter's case can be handled by the various options mentioned in comment 14 and comment 15.