628703 - Should not pass the ID of add-ons that have opted out of metadata pings to the discovery view

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Dave Townsend [:mossop]]

In bug 603409 we allowed add-ons to opt out of metadata pings but we are still passing the ID of these add-ons to the discovery view, we need to stop that from happening.

Comment 1

[Dave Townsend [:mossop]]

Attachment: patch rev 1

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 506917 [details] [diff] [review]
patch rev 1

>+        var prefName = "extensions." + aAddon.id + ".getAddons.cache.enabled";

Would much rather than this in a const, and replace a substring like we do for formatted URLs. Guess all that should be in the try..catch block too. 


>+        } catch (e) {
>+        }

Nit: I prefer no newline between } and { - makes it obvious that the catch{} block was intentionally left empty.


>+      if (!(aAddon.id in data)) {
>+        // Test add-ons will have shown an error if necessary above
>+        if (aAddon.id.substring(6) != "@tests.mozilla.org")
>+          ok(false, "Add-on " + aAddon.id + " was not included in the data");

Won't this trip up if there's a global extension installed, like the feedback extension?

Comment 3

[Dave Townsend [:mossop]]

(In reply to comment #2)
> Comment on attachment 506917 [details] [diff] [review]
> patch rev 1
> >+      if (!(aAddon.id in data)) {
> >+        // Test add-ons will have shown an error if necessary above
> >+        if (aAddon.id.substring(6) != "@tests.mozilla.org")
> >+          ok(false, "Add-on " + aAddon.id + " was not included in the data");
> 
> Won't this trip up if there's a global extension installed, like the feedback
> extension?

No, global extensions should be in the hash too so if we get here for a global extension then there is a test failure.

Comment 4

[Dave Townsend [:mossop]]

Attachment: backend patch rev 2

Comment 5

[Dave Townsend [:mossop]]

Landed: http://hg.mozilla.org/mozilla-central/rev/0f83f4ac8de4

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b12pre) Gecko/20110209 Firefox/4.0b12pre