718630 - Add-ons with compatibility override are displayed as enabled and incompatible if skipping compatibility check after upgrade

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Virgil Dicu [:virgil] [QA]]

Attachment: Add-on Manager Zotero listing

Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20100101 Firefox/10.0

The following add-ons now have compatibility overrides:
Zotero, versions 0-2.1.10, blocked in Firefox 10.0a1 - *
After the Deadline - Spell and Grammar Checker, version 1.50, blocked in Firefox 10.0a1 - * 

STR:
1. Start Firefox 9.0.1 with clean profile.
2. Install Zotero (blocked in Firefox 10.0a1 - * ) https://addons.mozilla.org/en-US/firefox/addon/zotero/?src=search
3. Upgrade to Firefox 10Beta4.
4. Start Firefox.
5. Skip the compatibility check at startup.
6. Restart the Add-ons manager.

Zotero is displayed as enabled in the Add-ons Manager with the incompatible add-on message.

Comment 1

[Virgil Dicu [:virgil] [QA]]

Attachment: Error Console messages

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

So, this is happening when the compatibility check is skipped (canceled) after the metadata ping starts, but before it finishes. The metadata check happens before update checks of the addons, which take the most time. So you have to cancel the dialog soon after it first shows, and it's a short window of opportunity (longer for slow connections).



Technical details:
Because the dialog does away, the callback is never executed when the metadata has been loaded and inserted into the DB - therefore the Add-ons Manager is never notified of the compatibility overrides (updateAddonRepositoryData is never called).

After startup, the UI correctly shows the addon as incompatible because that's recalculated whenever it's requested. But the value of Addon.appDisabled is based on information known before the compat overrides were seen.

When the dialog is dismissed, we could all AddonRepository.cancelSearch(), but there's still a problem if the dialog is dismissed after the metadata loads, but before the DB insertion completes. That may be good enough though, but only if we assume DB access isn't slow.

More foolproof alternatives:
* If the metadata check has started but not finished, keep the dialog open until it finishes. On very slow connections, this could potentially keep the dialog open for a noticeable period of time.
* Split out the fetching/caching parts in AddonRepository, so fetching can be canceled, and wait until the DB operations are done before closing the dialog.


Mossop: Any thoughts?

Comment 3

[Dave Townsend [:mossop]]

Nothing good. I guess another option would be to just assume everything is incompatible (i.e. enable strictCompatibility) until we get a response to the metadata ping. That would at least leave us failing safe. Other than that I'd probably just block the window closing till we get a response I guess.

Comment 4

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

Alas I don't think this can be tested reliably, since AFAIK there's no way to introduce a delay in the HTTP server's response (without a delay, there's a race condition).

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/78a838663c7d

Comment 6

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/78a838663c7d

Comment 7

[Virgil Dicu [:virgil] [QA]]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120226 Firefox/13.0a1
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:13.0) Gecko/20120226 Firefox/13.0a1
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120226 Firefox/13.0a1

Verified on latest Nightly with Mac OS 10.6, Ubuntu 11.10 and Windows 7. Manually upgraded (pave-over) from 8.0.1 to latest nightly. Zotero add-on was correctly disabled and displayed as incompatible.

To add the litmus test case after the merge to Aurora.

Comment 8

[Virgil Dicu [:virgil] [QA]]

https://litmus.mozilla.org/show_test.cgi?id=53246

Added new test case to litmus in Aurora-Silent Updates-Default to Compatible category.