Closed Bug 1239949 Opened 4 years ago Closed 4 years ago

Add support in repack script to show bookmark toolbar by default in partner repacks

Categories

(Release Engineering :: Custom Release Requests, defect)

defect
Not set

Tracking

(firefox47 fixed)

RESOLVED FIXED
Tracking Status
firefox47 --- fixed

People

(Reporter: hectorz, Unassigned)

References

Details

Attachments

(3 files)

Similar to bug 491477, this can be done by placing the attached localstore.rdf into browser/defaults/profile.

With this change, it'll be possible to create Fx China repack (distributed by Beijing office) for Mac OS X & Linux with partner-repack script.

I'll prepare a patch later.
Hi Hector,
Anything we add in repacks needs to remain contained in the distribution directory, otherwise we leave artifacts behind after uninstall on windows.  We have an addon that will open toolbar.

Dolske, we have enough partners and repacks doing this, can we build in a simple pref work for opening the toolbar on firstrun?  It would save the effort of using an addon or localstore hack.
Flags: needinfo?(dolske)
I don't know how widely it's used - mconnor would.

A pref hack sounds ok to me.
Flags: needinfo?(dolske) → needinfo?(mconnor)
(In reply to Shane Caraveo (:mixedpuppy) from comment #1)
> Hi Hector,
> Anything we add in repacks needs to remain contained in the distribution
> directory, otherwise we leave artifacts behind after uninstall on windows. 
> We have an addon that will open toolbar.

Just saw bug 1233837 where the override.ini hack was superseded by similar entry in distribution.ini.

> 
> Dolske, we have enough partners and repacks doing this, can we build in a
> simple pref work for opening the toolbar on firstrun?  It would save the
> effort of using an addon or localstore hack.

(In reply to Justin Dolske [:Dolske] from comment #2)
> I don't know how widely it's used - mconnor would.
> 
> A pref hack sounds ok to me.

Since people might be using localstore.rdf to customize other aspects of Fx UI, how do you like the idea of copying a localstore.rdf or xulstore.json from distribution directory to {ProfD} during XULStore.import at [1] if {ProfD}/localstore.rdf doesn't exist?

[1]: https://dxr.mozilla.org/mozilla-central/rev/584870f1cbc5/toolkit/components/xulstore/XULStore.js#93
Duplicate of this bug: 1249776
We have multiple partners that need the bookmark toolbar functionality, if it makes more sense to merge a xulstore.json that is a fine way to go.
Flags: needinfo?(mconnor)
I personally would rather see this as a preference. That would make it much easier for enterprises to do this as well. I'm concerned that xulstore.json is an implementation detail that partners and other folks shouldn't have to know about.
Attached patch Possible patchSplinter Review
I was thinking something like this.

I threw the menubar code in there just for fun as well.
(In reply to Mike Kaply [:mkaply] from comment #7)
> Created attachment 8722169 [details] [diff] [review]
> Possible patch
> 
> I was thinking something like this.
> 
> I threw the menubar code in there just for fun as well.

That looks reasonable.  Can this go into distributions.js instead (since it is repack specific)?
(In reply to Shane Caraveo (:mixedpuppy) from comment #8)
> (In reply to Mike Kaply [:mkaply] from comment #7)
> > Created attachment 8722169 [details] [diff] [review]
> > Possible patch
> > 
> > I was thinking something like this.
> > 
> > I threw the menubar code in there just for fun as well.
> 
> That looks reasonable.  Can this go into distributions.js instead (since it
> is repack specific)?

In theory yes. The distribution code is called before migrateUI, so we can test for the presence of browser.migration.version.

I wasn't sure if we wanted to put this kind of code in distribution.js, but I agree it makes the most sense.

I thought about that and I wasn't sure if we wanted to mess with xulstore in there.
Comment on attachment 8723031 [details]
MozReview Request: Bug 1239949 - Add a pref for showing bookmarks toolbar and menubar; r?mixedpuppy

https://reviewboard.mozilla.org/r/36315/#review32913
Attachment #8723031 - Flags: review?(mixedpuppy) → review+
https://hg.mozilla.org/integration/fx-team/rev/bd7f29886069642dfde5049cc5b399b715fe24ca
Bug 1239949 - Add a pref for showing bookmarks toolbar and menubar; r=mixedpuppy
https://hg.mozilla.org/mozilla-central/rev/bd7f29886069
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Thanks!
Blocks: 1249804
Duplicate of this bug: 724547
You need to log in before you can comment on or make changes to this bug.