1212590 - "Check for updates" doesn't update the about:addons extension view

Status: RESOLVED INACTIVE

(Toolkit :: Add-ons Manager, defect)

Description

[Vladan Djeric (:vladan)]

Attachment: addonManager.log

STR:
0. Have an out-of-date, unsigned addon in your profile
1. Open about:addons
2. Double-click on the out-of-date addon to examine it in detail (only this addon will be visible in about:addons)
3. Manually trigger an extension update check via "Check for updates"

Expected: Firefox installs an updated version of the addon, and immediately updates the addon's version & signed status in the about:addons view

Actual: Firefox does install an updated version of the addon, but the view of the extension doesn't update (still shows old version # and unsigned status). It's necessary to navigate back to the default about:addons view to see that anything was updated

I attached the Addon Manager log in case it helps

Comment 2

[Tony Mechelynck [:tonymec]]

IIUC this bug applies only to restartless add-ons? For old-style "restartful" add-ons, the add-on isn't actually updated until the browser is restarted, and by that time I'd expect the view to be reloaded.

Vladan: As a workaround (not a fix, since I agree that the add-on details ought to be updated together with the add-on itself) does hitting Ctrl+R (instead of going back to the list view) make the problem disappear?

Comment 3

[Tony Mechelynck [:tonymec]]

ah, duplicate bug 1231104 says yes ("…until the site gets refreshed").

Comment 4

[Edgar Lee [:hinshun]]

> University of Toronto, CSC302 assignment <

Hi there, I would like to work on this for my first Firefox contribution.

I have checked out the gecko-dev source, built Nightly 47.0a1 (2016-02-23) and able to run "./mach run".

1) Could you please guide me on where to look in the source code that manages extension updates?
2) For old-style "restartful" add-ons, is it expected behaviour to only update upon browser restart? (i.e. should I detect old-style "restartful" add-ons and leave them be?)
3) Any other tips/advice?

Many thanks.

Comment 5

[Edgar Lee [:hinshun]]

On Nightly 47.0a1 (2016-02-23):
I installed an out-of-date add-on using a .xpi, but I was unable to update the add-on in the detailed view without leaving to a separate updates view. After an update, click 'More' again lead me to an updated view with the correct versioning.

I realized the bug report was due for "Version: 42 Branch", does this mean I should work off GECKO420_2015102918_RELBRANCH?

Thanks again.

Comment 6

[Dave Townsend [:mossop]]

(In reply to Edgar Lee [:hinshun] from comment #5)
> On Nightly 47.0a1 (2016-02-23):
> I installed an out-of-date add-on using a .xpi, but I was unable to update
> the add-on in the detailed view without leaving to a separate updates view.
> After an update, click 'More' again lead me to an updated view with the
> correct versioning.

You need to click the gear button at the top of the page and select check for updates. That should allow you to reproduce this bug.

The code handling this view is here: https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/content/extensions.js#2843

You probably just need to add an onInstalled listener to call updateState but we'll also want an automated test to cover this. See https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/test/browser/browser_details.js for some existing tests for this view.

Thanks for helping out!

Comment 7

[Edgar Lee [:hinshun]]

(In reply to Dave Townsend [:mossop] from comment #6)
> You need to click the gear button at the top of the page and select check
> for updates. That should allow you to reproduce this bug.
> 
> The code handling this view is here:
> https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/
> content/extensions.js#2843
> 
> You probably just need to add an onInstalled listener to call updateState
> but we'll also want an automated test to cover this. See
> https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/
> test/browser/browser_details.js for some existing tests for this view.
> 
> Thanks for helping out!

Thank you for the help, I managed to reproduce the bug. I spent a couple of hours trying to figure out the solution, but I encountered some problems because of some asynchronous nature of the add-on wrappers.

I started with the onInstalled listener, but it seems like it receives no arguments, so updateState is still internally using the old add-on and it doesn't seem to have an effect.

Next, I went tried to use onInstallEnded(aInstall, aAddon) which seems like it receives an aAddon that is not yet fully resolved? When I set a breakpoint, passing it to _updateView seems to do the trick so I wrapped it with a call to AddonManager.getAddonByID to get a resolved version. However, this resolved version's optionURL has a value of "data:text/xml,<placeholder/>", so when it later attempts to fill in the settings, it gets a XML document with no settings.

Am I going towards the right path? The view updating works fine, but its only missing the settings now. I wonder if I should just try to reload the page instead.

Comment 8

[Dave Townsend [:mossop]]

Tagging myself so I remember to reply to this tomorrow

Comment 9

[Edgar Lee [:hinshun]]

Thank you, just to add to my previous comment, the following is my current understanding of the situation:

In order for the view to display the add-on's settings made by its author, the preferences dictionary in the add-on's `package.json` is parsed and a XML document `options.xul` is dynamically generated as a resource for `aAddon` from the work of bug 903039. This document then is referenced by a property called `optionsURL` to which a GET XMLHttpRequest is sent to retrieve the resource to populate the view.

The `aAddon` instance in `extensions.js` is of inline options type, which means that the program assumes the options resource is embedded with the add-on instance. But the newly updated add-on instance does not have an inline options resource, which means that the settings cannot be generated.

A simpler but workaround is a simple one-liner `window.location.reload()` in an onInstalled event handler, but I'm not sure if that is too hacky. I have attached my current work to kickstart this process.

Comment 10

[Edgar Lee [:hinshun]]

Attachment: 0001-Bug-1212590-Check-for-updates-doesn-t-update-the-abo.patch

Working patch that correctly updates the view, but add-on settings are not generated due to placeholder options on the `aAddon` instance.

Comment 11

[Dave Townsend [:mossop]]

(In reply to Edgar Lee [:hinshun] from comment #9)
> Thank you, just to add to my previous comment, the following is my current
> understanding of the situation:
> 
> In order for the view to display the add-on's settings made by its author,
> the preferences dictionary in the add-on's `package.json` is parsed and a
> XML document `options.xul` is dynamically generated as a resource for
> `aAddon` from the work of bug 903039. This document then is referenced by a
> property called `optionsURL` to which a GET XMLHttpRequest is sent to
> retrieve the resource to populate the view.
> 
> The `aAddon` instance in `extensions.js` is of inline options type, which
> means that the program assumes the options resource is embedded with the
> add-on instance. But the newly updated add-on instance does not have an
> inline options resource, which means that the settings cannot be generated.

Thanks, you've done a lot of good investigation here to understand what is going on which is great because at first glance I'd assume your patch would work perfectly.

The problem you describe is specific to add-ons built with the add-on SDK, other add-ons aren't affected. For those cases does the page display otherwise normally just with no options or does anything break? If it doesn't break I'd be inclined to just move forward with your patch and then file a follow-up bug to figure out how to fix the SDK case. This is because it leaves us materially better by showing the correct add-on info (and no longer showing outdated options) regardless. The SDK fix might be made in the SDK too so it is better handled as a separate issue.

In that case you shouldn't even need the getAddonByID, just passing the new add-on to updateView should work but then we need tests to verify the fix.

Comment 12

[Kris Maglione [:kmag]]

Hi Edgar,

Are you still interested in working on this? Do you have any more questions?

Thanks!

Comment 13

[Feng Guo]

Can I take a look on this bug? thanks

Comment 14

[Kris Maglione [:kmag]]

(In reply to Feng Guo from comment #13)
> Can I take a look on this bug? thanks

Please do. Do you have any questions?

Comment 15

[Feng Guo]

Hi Kris, thanks for quick reply, just wondering where can I get the out of date addons for testing?

Comment 16

[Kris Maglione [:kmag]]

Add-ons hosted on addons.mozilla.org have a version history page where you can download older versions. E.g.,

https://addons.mozilla.org/addon/adblock-plus/versions/

Comment 17

[Feng Guo]

(In reply to Kris Maglione [:kmag] from comment #16)
> Add-ons hosted on addons.mozilla.org have a version history page where you
> can download older versions. E.g.,
> 
> https://addons.mozilla.org/addon/adblock-plus/versions/
Hi Kris, 

I try to reproduce this bug from the previous comments. So,I try to update and it shows "available update" and after update reopen the details was fine. However, if I have separate windows which the first window in details will not update without refresh. If that what you mean for the bug? Also, should I looking at the glistview function or gDetailView? thanks

Comment 18

[Kris Maglione [:kmag]]

(In reply to Feng Guo from comment #17)
> I try to reproduce this bug from the previous comments. So,I try to update
> and it shows "available update" and after update reopen the details was
> fine. However, if I have separate windows which the first window in details
> will not update without refresh. If that what you mean for the bug? Also,
> should I looking at the glistview function or gDetailView? thanks

I think the problem only happens when you check for updates while the details page is open. Although it's the same issue you would see if you had two about:addons tabs open, yes.

Comment 19

[Feng Guo]

(In reply to Kris Maglione [:kmag] from comment #18)
> (In reply to Feng Guo from comment #17)
> > I try to reproduce this bug from the previous comments. So,I try to update
> > and it shows "available update" and after update reopen the details was
> > fine. However, if I have separate windows which the first window in details
> > will not update without refresh. If that what you mean for the bug? Also,
> > should I looking at the glistview function or gDetailView? thanks
> 
> I think the problem only happens when you check for updates while the
> details page is open. Although it's the same issue you would see if you had
> two about:addons tabs open, yes.

Hi, I try to add the listener to update the addon status and wondering which function should I change? The previous comment was point to line 2843 the getListItemForID is that correct?

Comment 20

[Andy McKay]

Redirecting over to :aswan

Comment 21

[Andrew Swan [:aswan]]

Feng Guo, its been over a year, are you still interested in working on this?

Comment 22

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.