mozprofile should provide an option to install addons above the "staged" dir

ASSIGNED
Assigned to

Status

Testing
Mozbase
ASSIGNED
5 years ago
4 years ago

People

(Reporter: nodeless, Assigned: nodeless)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20100101 Firefox/22.0 (Beta/Release)
Build ID: 20130618035212

Steps to reproduce:

At some point, mozprofile was changed to install addons to "extensions/staged/" instead of "extensions/". Jetpack needs extensions to be installed into the latter, to enable functionality in bug 854937. A boolean option should be provided to enable this.
(Assignee)

Comment 1

5 years ago
Created attachment 780886 [details] [diff] [review]
0001-mozprofile-add-use_staged_dir-option-to-Profile-cons.patch
Really? I thought the Add-ons manager is moving those XPI files into place. Dave, would you mind to give feedback for the current behavior?
Flags: needinfo?(dtownsend+bugmail)
(In reply to nodeless from comment #0)
> User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20100101
> Firefox/22.0 (Beta/Release)
> Build ID: 20130618035212
> 
> Steps to reproduce:
> 
> At some point, mozprofile was changed to install addons to
> "extensions/staged/" instead of "extensions/". Jetpack needs extensions to
> be installed into the latter, to enable functionality in bug 854937. A
> boolean option should be provided to enable this.

Is that actually the case? What fails if we just put it into extensions/staged?
Flags: needinfo?(dtownsend+bugmail)
(Assignee)

Comment 4

5 years ago
This test https://github.com/nodeless/addon-sdk/blob/master/test/test-self.js#L38 fails if it's put into extensions/staged. I.e., addons dropped into the staged directory are given startup reason code STARTUP on next start, when INSTALL is expected.

What's the difference between the 'extensions' directory and 'extensions/staged'? If their behavior should be consistent, it would make more sense to fix this upstream.
Oh right this is kind of a strange case. The only time when Firefox expects to find a restartless add-on in the staged directory at startup is when upgrading an add-on from a previous version that wasn't restartless.

Really the expected way to install extensions from outside the app is by dropping them into the extensions directory, dropping them into the staged directory is just a way we figured out we could bypass the opt-in screen on startup. We could also do that by setting a pref to disable the opt-in screen.

This is something that could be patched in Firefox, but I'm not sure how easily.
(Assignee)

Comment 6

5 years ago
Mozprofile used to use the 'extensions' directory, but was changed to 'staged' for bug 746243. Reading through that bug, I'm unclear on why mochitest needs to put their extensions into 'staged'. From your comment, I'm assuming it's because "dropping them into the staged directory is just a way we figured out we could bypass the opt-in screen on startup". I don't know what this "opt-in screen" is though, and why it isn't seen when running the jetpack test suite.
(In reply to nodeless from comment #6)
> Mozprofile used to use the 'extensions' directory, but was changed to
> 'staged' for bug 746243. Reading through that bug, I'm unclear on why
> mochitest needs to put their extensions into 'staged'. From your comment,
> I'm assuming it's because "dropping them into the staged directory is just a
> way we figured out we could bypass the opt-in screen on startup". I don't
> know what this "opt-in screen" is though, and why it isn't seen when running
> the jetpack test suite.

The opt-in screen is shown any time Firefox detects an add-on has been installed by something else on the system (https://blog.mozilla.org/addons/files/2011/08/3rdparty-install.png). For jetpack we disable this for extensions in the profile by setting a pref: https://github.com/mozilla/addon-sdk/blob/master/python-lib/cuddlefish/prefs.py#L34
(Assignee)

Comment 8

5 years ago
(In reply to Dave Townsend (:Mossop) from comment #7)
> (In reply to nodeless from comment #6)
> > Mozprofile used to use the 'extensions' directory, but was changed to
> > 'staged' for bug 746243. Reading through that bug, I'm unclear on why
> > mochitest needs to put their extensions into 'staged'. From your comment,
> > I'm assuming it's because "dropping them into the staged directory is just a
> > way we figured out we could bypass the opt-in screen on startup". I don't
> > know what this "opt-in screen" is though, and why it isn't seen when running
> > the jetpack test suite.
> 
> The opt-in screen is shown any time Firefox detects an add-on has been
> installed by something else on the system
> (https://blog.mozilla.org/addons/files/2011/08/3rdparty-install.png). For
> jetpack we disable this for extensions in the profile by setting a pref:
> https://github.com/mozilla/addon-sdk/blob/master/python-lib/cuddlefish/prefs.
> py#L34

Ah okay, I think I see what happened then. When mozbase was being integrated into mochitest, they didn't set the autoDisableScopes option, so firefox wouldn't pick up the addons. To work around it, they used the 'staged' dir. From bug 746243: "I had to [...] add extensions into the extensions/staged directory in order for them to be picked up by firefox."

I think we should kill this patch then, and instead revert line 207 of mozprofile/mozprofile/addons.py from patch https://github.com/mozilla/mozbase/commit/e5c9cea7956ee0a1efbe10bb07e2d7f73b6e72cd. Then we should chime in on bug 835930 to let them know to set the autoDisableScopes preference.

Does this sound like the right path forward?
(Assignee)

Updated

5 years ago
Blocks: 897370
(In reply to nodeless from comment #8)
> (In reply to Dave Townsend (:Mossop) from comment #7)
> > (In reply to nodeless from comment #6)
> > > Mozprofile used to use the 'extensions' directory, but was changed to
> > > 'staged' for bug 746243. Reading through that bug, I'm unclear on why
> > > mochitest needs to put their extensions into 'staged'. From your comment,
> > > I'm assuming it's because "dropping them into the staged directory is just a
> > > way we figured out we could bypass the opt-in screen on startup". I don't
> > > know what this "opt-in screen" is though, and why it isn't seen when running
> > > the jetpack test suite.
> > 
> > The opt-in screen is shown any time Firefox detects an add-on has been
> > installed by something else on the system
> > (https://blog.mozilla.org/addons/files/2011/08/3rdparty-install.png). For
> > jetpack we disable this for extensions in the profile by setting a pref:
> > https://github.com/mozilla/addon-sdk/blob/master/python-lib/cuddlefish/prefs.
> > py#L34
> 
> Ah okay, I think I see what happened then. When mozbase was being integrated
> into mochitest, they didn't set the autoDisableScopes option, so firefox
> wouldn't pick up the addons. To work around it, they used the 'staged' dir.
> From bug 746243: "I had to [...] add extensions into the extensions/staged
> directory in order for them to be picked up by firefox."
> 
> I think we should kill this patch then, and instead revert line 207 of
> mozprofile/mozprofile/addons.py from patch
> https://github.com/mozilla/mozbase/commit/
> e5c9cea7956ee0a1efbe10bb07e2d7f73b6e72cd. Then we should chime in on bug
> 835930 to let them know to set the autoDisableScopes preference.
> 
> Does this sound like the right path forward?

I think that is the right path, except we'll need to get the other test harnesses updated to set that pref before we can change the mozprofile behaviour.
(Assignee)

Comment 10

5 years ago
Upon further inspection, it appears mozprofile sets the autoDisableScopes preference correctly itself (https://github.com/mozilla/mozbase/commit/dfeb07adfd0883b0a14c3c9497011a06b2009e14). That commit was made months before mochitest opened their bug to port to mozbase, so they would have been using that version. In that case, I'm not sure why mochitest would've had trouble getting their addons picked up as described in https://bugzilla.mozilla.org/show_bug.cgi?id=746243#c3. I just tested the addon-sdk without setting autoDisableScopes (using new mozbase, so that it would set the pref itself), and it works, so I think that we can revert the 'staged' change without breaking anyone. I'd like to confirm that this hypothesis is correct though.

Andrew, I saw that you built the patch to move mochitest to mozbase in bug 835930. Would you be able to spare a moment and delete the 'staged' argument from line 223 and 258 of mozbase (https://github.com/mozilla/mozbase/blob/master/mozprofile/mozprofile/addons.py#L223) and see if mochitest still works (specifically, that firefox picks up addons that it installs via profile)? The 'staged' argument was added to support mochitest (bug 746243), but I think it's actually not required. We want to revert it because it breaks the addon-sdk, but first I want to confirm that this doesn't break downstream consumers.
Flags: needinfo?(ahalberstadt)
I only did the b2g mochitests, though I think desktop mochitests started using mozprofile recently as well.

Here's a try run: https://tbpl.mozilla.org/?tree=Try&rev=b32ea72b2510
Flags: needinfo?(ahalberstadt)
That didn't work, removed all references to staged in the mochitest runtests files:
https://tbpl.mozilla.org/?tree=Try&rev=c40353334740
(Assignee)

Comment 13

5 years ago
Created attachment 787332 [details] [diff] [review]
mochitest_staged.patch

That one didn't work either. Can you submit the attached as a try run? It changes the automation code to use mozprofile.FirefoxProfile instead of mozprofile.Profile. It needs that because autoDisableScopes preference is set in FirefoxProfile. It fixes the timeout problem in runtests.py (on my win7 box).
If we need autoDisableScopes in other applications (b2g) as well, shouldn't it get moved to the base profile class then?

nodeless, I assume you want to work on this bug given the awesome investigation of the problem. So I will assign this bug to you.
Assignee: nobody → nodeless
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
What work remains to be done here?
You need to log in before you can comment on or make changes to this bug.