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

Status: RESOLVED FIXED

(Release Engineering :: Release Requests, defect)

Description

[Hector Zhao [:hectorz]]

Attachment: localstore.rdf to show bookmark toolbar by default

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.

Comment 1

[Shane Caraveo (:mixedpuppy)]

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.

Comment 2

[Justin Dolske [:Dolske]]

I don't know how widely it's used - mconnor would.

A pref hack sounds ok to me.

Comment 3

[Hector Zhao [:hectorz]]

(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

Comment 5

[Shane Caraveo (:mixedpuppy)]

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.

Comment 6

[Mike Kaply [:mkaply]]

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.

Comment 7

[Mike Kaply [:mkaply]]

Attachment: Possible patch

I was thinking something like this.

I threw the menubar code in there just for fun as well.

Comment 8

[Shane Caraveo (:mixedpuppy)]

(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)?

Comment 9

[Mike Kaply [:mkaply]]

(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 10

[Mike Kaply [:mkaply]]

Attachment: MozReview Request: Bug 1239949 - Add a pref for showing bookmarks toolbar and menubar; r?mixedpuppy

Review commit: https://reviewboard.mozilla.org/r/36315/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/36315/

Comment 11

[Shane Caraveo (:mixedpuppy)]

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

Comment 12

[Mike Kaply [:mkaply]]

https://hg.mozilla.org/integration/fx-team/rev/bd7f29886069642dfde5049cc5b399b715fe24ca
Bug 1239949 - Add a pref for showing bookmarks toolbar and menubar; r=mixedpuppy

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bd7f29886069

Comment 14

[Hector Zhao [:hectorz]]

Thanks!