335238 - update requests for disabled extensions should have some "disabled" indicator in the request

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

When we fetch update information for a disabled extension, it would be good to have some way of telling on the server side that the extension was disabled.

This would be useful information for extension authors, and would also help us make sure that blacklisted extensions are being treated appropriately.

We could put it in the URL via another %EXT_STATUS% substitution, or add an X-Mozilla-Extension-Status: header to the request, as two options that come immediately to mind.

Comment 1

[Robert Strong (they/them - no direct email)]

There are two types of disabled props - app disabled and user disabled. They are independent of each other so it may be best to send both.

Comment 2

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

We could do a comma-separated set of status flags, or something:

status=ok
status=appdisabled
status=appdisabled,userdisabled
status=blacklisted

(or more compact versions of those, if desired)

Comment 3

[Robert Strong (they/them - no direct email)]

This is trivial to add and I'll try to do so in the next couple of days if I can get / come up with a finalized set of codes to return. An add-on that is blocklisted is also appDisabled so instead of returning appDisabled how about returning why it is appDisabled?

status=enabled
status=userDisabled

These 3 could potentially include userDisabled
status=blocklisted
status=incompatible
status=needsDependencies

Thoughts?

Comment 4

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Those sound fine.  I presume we know all the reasons that an extension can be disabled (from our test suite for this, of course ;) ), but we can add new entries to that list later if we need, right?

Comment 5

[Robert Strong (they/them - no direct email)]

taking before I forget about this

Comment 6

[Dave Townsend [:mossop]]

Attachment: patch rev 1

Adds a %ITEM_STATUS% field to the update urls, made up as specified in comment 3. Also adds this field into the default update URL.

Comment 7

[Robert Strong (they/them - no direct email)]

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

>Index: toolkit/mozapps/extensions/src/nsExtensionManager.js.in
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v
>retrieving revision 1.198
>diff -u -8 -p -r1.198 nsExtensionManager.js.in
>--- toolkit/mozapps/extensions/src/nsExtensionManager.js.in	15 Jun 2006 21:32:21 -0000	1.198
>+++ toolkit/mozapps/extensions/src/nsExtensionManager.js.in	18 Jun 2006 17:58:48 -0000
>@@ -6073,32 +6073,47 @@ RDFItemUpdater.prototype = {
...
>+    var itemStatus;
>+    if (emDS.getItemProperty(aItem.id, "userDisabled") == "true" ||
>+        emDS.getItemProperty(aItem.id, "userDisabled") == OP_NEEDS_ENABLE)
>+      itemStatus = "userDisabled";
>+    else
>+      itemStatus = "enabled";
After looking at it I think this would makes more sense if it were userEnabled.

r=me with that change.

Comment 8

[Dave Townsend [:mossop]]

Attachment: patch rev 2

Addressed comment, carrying over review, ready for checkin.

Comment 9

[Robert Strong (they/them - no direct email)]

Checked in on trunk. I'm going to let this bake for a couple of days before approving for 1.8.1. Thanks!

Comment 10

[Robert Strong (they/them - no direct email)]

I decided this is safe to land so checked in to MOZILLA_1_8_BRANCH for Firefox 2.0

Comment 11

[Alex Ihrig [:Thunderbird_Mail_DE]]

Robert your checkin for bug 307358 extensions.properties rev 1.27 removes the changes made to extensions.update.url in rev 1.26 (only in trunk - not in 1.8_Branch).

Comment 12

[Robert Strong (they/them - no direct email)]

(In reply to comment #11)
Thanks and fixed.