Closed Bug 718630 Opened 9 years ago Closed 9 years ago

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

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla13

People

(Reporter: virgil.dicu, Assigned: Unfocused)

References

Details

(Whiteboard: [qa!])

Attachments

(3 files)

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.
Attached file Error Console messages
OS: Linux → All
Hardware: x86_64 → All
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?
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.
Blocks: 693906
Attached patch Patch v1Splinter Review
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).
Assignee: nobody → bmcbride
Status: NEW → ASSIGNED
Attachment #592615 - Flags: review?(dtownsend)
Attachment #592615 - Flags: review?(dtownsend) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/78a838663c7d
Flags: in-testsuite-
Flags: in-litmus?
Target Milestone: --- → mozilla13
Flags: in-litmus? → in-litmus?(virgil.dicu)
https://hg.mozilla.org/mozilla-central/rev/78a838663c7d
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → VERIFIED
Whiteboard: [qa!]
Depends on: 734822
https://litmus.mozilla.org/show_test.cgi?id=53246

Added new test case to litmus in Aurora-Silent Updates-Default to Compatible category.
Flags: in-litmus?(virgil.dicu) → in-litmus+
You need to log in before you can comment on or make changes to this bug.