If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Need to check for add-on updates

RESOLVED FIXED

Status

Fennec Graveyard
General
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: mfinkle, Assigned: mfinkle)

Tracking

Trunk
x86
Mac OS X

Details

Attachments

(1 attachment, 2 obsolete attachments)

Manual and automatic checks for add-on updates should be implemented
(Assignee)

Updated

8 years ago
tracking-fennec: --- → ?

Updated

8 years ago
tracking-fennec: ? → 1.0+

Updated

8 years ago
Assignee: nobody → mark.finkle
Looks like we will be settling for manual updates for Fennec 1.0

The flow will be:
* User presses the "Update" button (in the "Your Add-ons" header)
* Each add-on will be tested for an available update
  * Description in row changes to "Checking for updates"
* Any add-on with an update will begin to download the update
  * Description in row changes to "Updating"
* Add-ons complete update
  * Description in row changes to "Updated"
* If at least one add-on has been updated, the Restart notification will be shown
Should we give some indication if no updates are available?
(In reply to comment #2)
> Should we give some indication if no updates are available?

With the proposed design, every add-on would get the "Checking for update" message. If an update is not available, the message area would then be blank.

We could, instead, say "No update found"

Madhava?
(In reply to comment #3)

> With the proposed design, every add-on would get the "Checking for update"
> message. If an update is not available, the message area would then be blank.

Not blank. I guess it would revert back to the true description of the add-on.
Created attachment 398781 [details] [diff] [review]
WIP 1

This patch adds the strings and the code needed to perform an addon update. I need to do more testing to make sure the updates actually work.
Created attachment 398809 [details] [diff] [review]
patch

This patch is functional - it actually checks and downloads updates for add-ons. A few things I need to followup:

* The "Update" button needs to be styled better - it blends into the section header too much
* Sometimes, after finding a download, the "Restart" notification bar is not displayed, even though a manual restart will load the updated add-on

The strings are working and should provide enough context for localizers.
Attachment #398781 - Attachment is obsolete: true
Attachment #398809 - Flags: review?(gavin.sharp)
Screenshots of the before and after update check:
http://people.mozilla.com/~mfinkle/fennec/screenshots/fennec-addon-update-before.png
http://people.mozilla.com/~mfinkle/fennec/screenshots/fennec-addon-update-after.png
Created attachment 398867 [details] [diff] [review]
patch 2

Same as previous patch, but I move the "Updated to [version]" string change into XPInstallDownloadManager so we only set that string when we are finished with the download.

I added "Updating to [version]" string and use it when we know there is an update we can download.
Attachment #398809 - Attachment is obsolete: true
Attachment #398867 - Flags: review?(gavin.sharp)
Attachment #398809 - Flags: review?(gavin.sharp)
Comment on attachment 398867 [details] [diff] [review]
patch 2

Adding Mossop for the nitty gritty update code.

I might land before he has a chance to review (for string freeze) but it's a good idea to have him look at the code and give feedback.
Attachment #398867 - Flags: review?(dtownsend)
Comment on attachment 398867 [details] [diff] [review]
patch 2

>diff --git a/chrome/content/extensions.js b/chrome/content/extensions.js

>+  updateAll: function ev_updateAll() {

>+    this._pref.setBoolPref("extensions.update.notifyUser", false);

Why set this each time but never reset it? Can't you just set the default instead?
Attachment #398867 - Flags: review?(gavin.sharp) → review+
Looks like there is a background update listener in nsExtensionManager who has the job of setting this to true. Code in FF extensions.js only sets it to false.

I'll leave it as is for now. Maybe Dave can shed some light on the best way to deal with it.
pushed: https://hg.mozilla.org/mobile-browser/rev/732abc08a434

Need to file a bug on styling the "Update" button
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Comment on attachment 398867 [details] [diff] [review]
patch 2

>+  updateAll: function ev_updateAll() {
>+    if (!this._isXPInstallEnabled())
>+      return;
>+  
>+    // To support custom views we check the add-ons displayed in the list
>+    let items = [];
>+    let start = this._localItem.nextSibling;
>+    let end = this._repoItem;
>+
>+    while (start != end) {
>+      if (start.getAttribute("updateable") != "false")
>+        items.push(this._extmgr.getItemForID(start.getAttribute("addonID")));
>+      start = start.nextSibling;
>+    }
>+  
>+    if (items.length > 0) {
>+      let listener = new UpdateCheckListener();
>+      this._extmgr.update(items, items.length, Ci.nsIExtensionManager.UPDATE_CHECK_NEWVERSION, listener);
>+    }
>+    
>+    if (this._list.selectedItem)
>+      this._list.selectedItem.focus();
>+  
>+    this._pref.setBoolPref("extensions.update.notifyUser", false);

This pref change should be moved to be after the update check has completed in onUpdateEnded.

Also that thing we talked about on IRC might be more efficient, otherwise looks fine.
Attachment #398867 - Flags: review?(dtownsend) → review+
You need to log in before you can comment on or make changes to this bug.