Last Comment Bug 635115 - Allow opting out of sending add-on information to the discovery pane without disabling automatic add-on updates
: Allow opting out of sending add-on information to the discovery pane without ...
Status: VERIFIED FIXED
:
Product: Toolkit
Classification: Components
Component: Add-ons Manager (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla2.0
Assigned To: Dave Townsend [:mossop]
:
: Andy McKay [:andym]
Mentors:
http://www.mozilla.com/en-US/legal/pr...
Depends on:
Blocks: 639722 641537
  Show dependency treegraph
 
Reported: 2011-02-17 17:10 PST by Dave Townsend [:mossop]
Modified: 2011-03-22 07:20 PDT (History)
8 users (show)
dtownsend: in‑testsuite+
dtownsend: in‑litmus-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
patch rev 1 (12.89 KB, patch)
2011-02-20 11:23 PST, Dave Townsend [:mossop]
blair: review+
Details | Diff | Splinter Review
ready to land (12.88 KB, patch)
2011-02-23 17:35 PST, Dave Townsend [:mossop]
dtownsend: review+
mbeltzner: approval2.0+
Details | Diff | Splinter Review

Description Dave Townsend [:mossop] 2011-02-17 17:10:51 PST
We have an about:config pref to control globally disabling the metadata check but nothing similar for the discovery request.

Both are disabled by turning off background update checks entirely but we do not want users to do that wherever possible.
Comment 1 Henrik Skupin (:whimboo) 2011-02-18 00:40:26 PST
Asking for blocking because the current behavior is not what we explain to users on our privacy policy. See bug 624591 for the last related change to that given topic.

Quoting:

To display the personalized recommendations, Firefox sends certain information to Mozilla, including the list of add-ons you have installed, Firefox version information, and your IP address. This communication only happens when the Get Add-ons area is open and can be turned off at any time by opting out of Automatic Updates from the Add-ons Manager.
Comment 2 Mike Beltzner [:beltzner, not reading bugmail] 2011-02-18 09:15:43 PST
Uh, no way we block on changing this at this time - let's just change the website, instead, OK?

If a safe patch comes along, I'd be happy to approve it.
Comment 3 Dave Townsend [:mossop] 2011-02-18 11:11:22 PST
Agree we shouldn't block but there are some problematic inconsistencies here that it would make our lives easier in the future to resolve before Firefox 4. After detailing the current state in https://wiki.mozilla.org/Extension_Manager:Update_Checking#Background_Metadata_Check the final section lists the goals of the change. We can implement it with the following simple changes:

* Stop extension.update.enabled from controlling whether add-on info is sent to the discovery pane.
* Make extensions.getAddons.cache.enabled controll whether add-on info is sent to the discovery pane.
* Stop the background update install setting from controlling whether add-on info is included in the metadata request.

This is basically changing the pref used for the discovery pane control and backing out bug 629237.
Comment 4 Justin Scott [:fligtar] 2011-02-18 14:04:34 PST
Are you also making extensions.getAddons.cache.enabled control post-install metadata requests?
Comment 5 Dave Townsend [:mossop] 2011-02-18 14:08:26 PST
(In reply to comment #4)
> Are you also making extensions.getAddons.cache.enabled control post-install
> metadata requests?

It already does control those
Comment 6 Henrik Skupin (:whimboo) 2011-02-18 14:18:22 PST
Sounds all fine with me Dave! Thanks for writing up all of that. Even the verification of bug 629237 was a bit late, I'm glad we have found that problematic before the final release.
Comment 7 Dave Townsend [:mossop] 2011-02-20 11:23:15 PST
Created attachment 513878 [details] [diff] [review]
patch rev 1

Simple patch that makes the mentioned changes, also some test fixes because now by default the add-ons manager UI finishes initializing earlier than it did before. This has passed on try server.
Comment 8 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-02-23 17:23:07 PST
Comment on attachment 513878 [details] [diff] [review]
patch rev 1

>+      if ("getCachedAddonByID" in scope.AddonRepository) {
>+        pendingUpdates++;
>+        var ids = [a.id for each (a in aAddons)];
>+        scope.AddonRepository.repopulateCache(ids, notifyComplete);
>+      }

Nit: I realize this is just a straight backout of bug 629237, but the check for an implementation of getCachedAddonByID was intentionally removed there (since AddonRepository is no longer expected to have alternate implementations). Would like to see that check stay gone.


>diff --git a/toolkit/mozapps/extensions/content/extensions.js b/toolkit/mozapps/extensions/content/extensions.js
...
>-        var prefName = PREF_GETADDONS_CACHE_ENABLED.replace("%ID%", aAddon.id);
>+        var prefName = PREF_GETADDONS_CACHE_ID_ENABLED.replace("%ID%", aAddon.id);

Nit: Wrap to 80 chars.
Comment 9 Dave Townsend [:mossop] 2011-02-23 17:35:32 PST
Created attachment 514678 [details] [diff] [review]
ready to land

Updated from review comments. Requesting review as this will allow users to opt out of sending add-on details to AMO without turning off updates for add-ons. This is a pretty risk free change (most of it is just a backout of an earlier patch) and is covered by tests.
Comment 10 Mike Beltzner [:beltzner, not reading bugmail] 2011-02-23 18:00:33 PST
Comment on attachment 514678 [details] [diff] [review]
ready to land

a=beltzner, good privacy win, I expect your restartless Add-On soon!
Comment 11 Dave Townsend [:mossop] 2011-02-24 12:44:53 PST
Landed: http://hg.mozilla.org/mozilla-central/rev/cd17f857b8c0
Comment 12 Jim Jeffery not reading bug-mail 1/2/11 2011-02-24 16:43:17 PST
Is there no user UI, just the pref in about:config ?

extensions.getAddons.cache.enabled
Comment 13 Dave Townsend [:mossop] 2011-02-24 17:15:19 PST
(In reply to comment #12)
> Is there no user UI, just the pref in about:config ?
> 
> extensions.getAddons.cache.enabled

correct.
Comment 15 Henrik Skupin (:whimboo) 2011-03-22 07:20:43 PDT
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0) Gecko/20100101 Firefox/4.0

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