Last Comment Bug 696598 - Add default value for extensions.autoDisableScopes
: Add default value for extensions.autoDisableScopes
Status: VERIFIED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.7
Assigned To: Jens Hatlak (:InvisibleSmiley)
:
Mentors:
Depends on:
Blocks: 700162
  Show dependency treegraph
 
Reported: 2011-10-22 12:34 PDT by Jens Hatlak (:InvisibleSmiley)
Modified: 2011-11-07 21:19 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch [Checkin: comment 9] (4.34 KB, patch)
2011-10-22 13:20 PDT, Jens Hatlak (:InvisibleSmiley)
iann_bugzilla: review+
Details | Diff | Splinter Review
browser fix [Checkin: comment 14] (1.67 KB, patch)
2011-11-06 09:57 PST, Jens Hatlak (:InvisibleSmiley)
philip.chee: review+
Details | Diff | Splinter Review

Description Jens Hatlak (:InvisibleSmiley) 2011-10-22 12:34:45 PDT
Changeset 15e15fead382: Bug 693743: Some 3rd party add-ons are getting installed into the profile and are not disabled on start-up

// Disable add-ons installed into the shared user and shared system areas by
// default. This does not include the application directory. See the SCOPE
// constants in AddonManager.jsm for values to use here
pref("extensions.autoDisableScopes", 15);

The scope constants are here:
http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/mozapps/extensions/AddonManager.jsm#1258

Pref first introduced through:
Changeset eee41544cb84: Bug 476430: Disable third-party add-ons by default and offer them to the user
from which we're also missing the nsBrowserGlue.js part (which we might want to add here, too).
Comment 1 Jens Hatlak (:InvisibleSmiley) 2011-10-22 13:20:01 PDT
Created attachment 568907 [details] [diff] [review]
patch [Checkin: comment 9]

Porting patch, nsSuiteGlue.js part yet untested.
Comment 2 Philip Chee 2011-10-22 21:30:09 PDT
Does anybody know how this affects extensions delivered via {application}/distribution/extensions ?
Comment 3 Tony Mechelynck [:tonymec] 2011-10-24 17:56:37 PDT
(In reply to Philip Chee from comment #2)
> Does anybody know how this affects extensions delivered via
> {application}/distribution/extensions ?

I /think/ the bit of weight 4 (as in 15 but not in 10, the two values used in various Fx builds) excludes them. I hope I'm wrong but… I guess the guy to ask this from might be Mossop.
Comment 4 Dave Townsend [:mossop] 2011-10-24 19:57:56 PDT
It doesn't affect them at all
Comment 5 Ian Neal 2011-10-29 03:44:35 PDT
Comment on attachment 568907 [details] [diff] [review]
patch [Checkin: comment 9]

I presume with this patch that we now pass the relevant tests from toolkit/mozapps/extensions/test/xpcshell that were introduced by bug 693743 ?
Comment 6 Jens Hatlak (:InvisibleSmiley) 2011-10-30 00:55:51 PDT
(In reply to Ian Neal from comment #5)
> I presume with this patch that we now pass the relevant tests from
> toolkit/mozapps/extensions/test/xpcshell that were introduced by bug 693743 ?

No. test_startup.js still fails through its dependency head_addons.js which fails with:

TEST-UNEXPECTED-FAIL | /path/to/my/mozilla/seamonkey-central/mozilla/_tests/xpcshell/toolkit/mozapps/extensions/test/xpcshell/head_addons.js | [] == ["modern@themes.mozilla.org"] - See following stack:
JS frame :: /path/to/my/comm-central/mozilla/testing/xpcshell/head.js :: do_throw :: line 453
JS frame :: /path/to/my/comm-central/mozilla/testing/xpcshell/head.js :: _do_check_eq :: line 547
JS frame :: /path/to/my/comm-central/mozilla/testing/xpcshell/head.js :: do_check_eq :: line 568
JS frame :: /path/to/my/mozilla/seamonkey-central/mozilla/_tests/xpcshell/toolkit/mozapps/extensions/test/xpcshell/head_addons.js :: check_startup_changes :: line 454

which is probably because the test only excludes the default theme (which happens to have the same ID for both FF and SM) in check_startup_changes on line 450 but not Modern. I guess that can only be fixed in the test itself, but I'm not sure whether adding yet another static value is exactly the right way to go there... Probably easiest, though (if accepted). Dave, what would be my chances there if I filed a bug?
Comment 7 Dave Townsend [:mossop] 2011-10-31 10:34:51 PDT
(In reply to Jens Hatlak (:InvisibleSmiley) from comment #6)
> (In reply to Ian Neal from comment #5)
> > I presume with this patch that we now pass the relevant tests from
> > toolkit/mozapps/extensions/test/xpcshell that were introduced by bug 693743 ?
> 
> No. test_startup.js still fails through its dependency head_addons.js which
> fails with:
> 
> TEST-UNEXPECTED-FAIL |
> /path/to/my/mozilla/seamonkey-central/mozilla/_tests/xpcshell/toolkit/
> mozapps/extensions/test/xpcshell/head_addons.js | [] ==
> ["modern@themes.mozilla.org"] - See following stack:
> JS frame :: /path/to/my/comm-central/mozilla/testing/xpcshell/head.js ::
> do_throw :: line 453
> JS frame :: /path/to/my/comm-central/mozilla/testing/xpcshell/head.js ::
> _do_check_eq :: line 547
> JS frame :: /path/to/my/comm-central/mozilla/testing/xpcshell/head.js ::
> do_check_eq :: line 568
> JS frame ::
> /path/to/my/mozilla/seamonkey-central/mozilla/_tests/xpcshell/toolkit/
> mozapps/extensions/test/xpcshell/head_addons.js :: check_startup_changes ::
> line 454
> 
> which is probably because the test only excludes the default theme (which
> happens to have the same ID for both FF and SM) in check_startup_changes on
> line 450 but not Modern. I guess that can only be fixed in the test itself,
> but I'm not sure whether adding yet another static value is exactly the
> right way to go there... Probably easiest, though (if accepted). Dave, what
> would be my chances there if I filed a bug?

I think the right thing there is to filter out any add-ons with IDs that don't end with "@tests.mozilla.org".
Comment 8 Jens Hatlak (:InvisibleSmiley) 2011-10-31 11:41:39 PDT
(In reply to Dave Townsend (:Mossop) from comment #7)
> I think the right thing there is to filter out any add-ons with IDs that
> don't end with "@tests.mozilla.org".

Filed bug 698524.
Comment 9 Jens Hatlak (:InvisibleSmiley) 2011-11-06 02:20:53 PST
Comment on attachment 568907 [details] [diff] [review]
patch [Checkin: comment 9]

http://hg.mozilla.org/comm-central/rev/58837246ee0b
Comment 10 Philip Chee 2011-11-06 09:21:42 PST
> +    var win = this.getMostRecentBrowserWindow();
You forgot to define getMostRecentBrowserWindow() anywhere in nsSuiteGlue.js

Introduced in:
http://hg.mozilla.org/mozilla-central/rev/a131999fa900
Modified in:
http://hg.mozilla.org/mozilla-central/rev/36a5be172c91
http://hg.mozilla.org/mozilla-central/rev/3d137f5214ad
Comment 11 Philip Chee 2011-11-06 09:32:59 PST
>    _onBrowserStartup: function(aWindow)
And besides, you already have a window.

>    {
>      if (Services.prefs.getBoolPref("plugins.update.notifyUser"))
>        this._showPluginUpdatePage(aWindow);
>  
> +    // For any add-ons that were installed disabled and can be enabled offer
> +    // them to the user.
> +    var win = this.getMostRecentBrowserWindow();
> +    var browser = win.gBrowser;
Use getBrowser() like notifyBox below (our gBrowser isn't a lazy getter).

> +    var changedIDs = AddonManager.getStartupChanges(AddonManager.STARTUP_CHANGE_INSTALLED);
> +    AddonManager.getAddonsByIDs(changedIDs, function(aAddons) {
> +      aAddons.forEach(function(aAddon) {
> +        // If the add-on isn't user disabled or can't be enabled then skip it.
> +        if (!aAddon.userDisabled || !(aAddon.permissions & AddonManager.PERM_CAN_ENABLE))
> +          return;
> +
> +        browser.selectedTab = browser.addTab("about:newaddon?id=" + aAddon.id);
> +      })
> +    });
> +
>      var notifyBox = aWindow.getBrowser().getNotificationBox();
reuse |var browser| ?
Comment 12 Jens Hatlak (:InvisibleSmiley) 2011-11-06 09:57:07 PST
Created attachment 572309 [details] [diff] [review]
browser fix [Checkin: comment 14]

Fix "what Ratty said". Thanks for the heads-up!
Comment 13 Philip Chee 2011-11-06 10:32:26 PST
Comment on attachment 572309 [details] [diff] [review]
browser fix [Checkin: comment 14]

OK. Tested by dropping an extension into seamonkey/extensions/
r=me

p.s. please file a Themes->Modern bug to style the about:newaddon UI in modern.
Comment 14 Jens Hatlak (:InvisibleSmiley) 2011-11-06 11:31:52 PST
Comment on attachment 572309 [details] [diff] [review]
browser fix [Checkin: comment 14]

http://hg.mozilla.org/comm-central/rev/bd56a0704795
Comment 15 Tony Mechelynck [:tonymec] 2011-11-07 21:19:17 PST
The preference exists (default=15) in this build:
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a1) Gecko/20111107 Firefox/10.0a1 SeaMonkey/2.7a1 ID:20111107193531 (Mozilla tinderbox-build)

It wasn't listed in about:config in:
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a1) Gecko/20111106 Firefox/10.0a1 SeaMonkey/2.7a1 ID:20111106024605 (own-built build, timestamp corrected for timezone difference)

Note You need to log in before you can comment on or make changes to this bug.