Closed Bug 511593 Opened 11 years ago Closed 10 years ago

Need to check for add-on updates

Categories

(Firefox for Android Graveyard :: General, defect)

x86
macOS
defect
Not set

Tracking

(fennec1.0+)

RESOLVED FIXED
Tracking Status
fennec 1.0+ ---

People

(Reporter: mfinkle, Assigned: mfinkle)

Details

Attachments

(1 file, 2 obsolete files)

Manual and automatic checks for add-on updates should be implemented
tracking-fennec: --- → ?
tracking-fennec: ? → 1.0+
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.
Attached patch WIP 1 (obsolete) — Splinter Review
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.
Attached patch patch (obsolete) — Splinter Review
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)
Attached patch patch 2Splinter Review
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
Closed: 10 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.