Closed Bug 395430 Opened 14 years ago Closed 14 years ago

AdBlock Plus gets installed but is marked as incompatible after restart

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.9alpha8

People

(Reporter: u279076, Assigned: mossop)

References

Details

(Keywords: regression)

Attachments

(2 files)

Using Minefield 2007090704 on Linux, I can install AdBlock Plus from AMO but when I restart Minefield, the addon is marked as incompatible but no errors were given at all.

Steps to Reproduce:
1. Go to . and click on "install now" button
2. click install now from the Software Installation UI
3. AM comes up trying to install adblock
4. it downloads and checks compatibility
5. text below addon says "restart to complete installation"
6. click "restart minefield", minefield is restarted
7. after minefield comes back up, it behaves as if no addon was installed
8. check AM

Actual results:
Adblock says "not compatible with 3.0a8pre" with a button to uninstall.  No errors are displayed throughout.  Click on uninstall works as expected.

Expected results:
If the addon is indeed incompatible, this should be found during the compatibility check.
sorry...step 1 should read:

1. Go to AMO, find the AdBlock Plus addon page and click on "install now" button
I know what's causing this, have a patch in progress.
Flags: in-litmus?
Attached patch patch rev 1Splinter Review
So we install the extension, detect new compatibility info to the datasource to be used on startup.

On restart we retrieve the updated compatibility info from the datasource, but since we fail to return the appid for the updated info, when we pass that back in to updateTargetAppInfo it does nothing with the new info, leaving the extension incompatible and disabled.
Assignee: nobody → dtownsend
Status: NEW → ASSIGNED
Attachment #280108 - Flags: review?(robert.bugzilla)
This is a regression from bug 299716. I have a testcase that is close to covering this that I can adapt but for the moment would like to try to get this fix into the tree a.s.a.p.
Flags: in-testsuite?
I'd r+ that in a heartbeat if so empowered.  Pretty obvious.
Attachment #280108 - Flags: review?(robert.bugzilla) → review+
Comment on attachment 280108 [details] [diff] [review]
patch rev 1

Requesting approval for this extremely simple patch that corrects extension installation in certain cases. There is I believe a pretty much non-existent risk of regressions from this
Attachment #280108 - Flags: approval1.9?
Comment on attachment 280108 [details] [diff] [review]
patch rev 1

yeah, let's get this in...
Attachment #280108 - Flags: approval1.9? → approval1.9+
Checking in toolkit/mozapps/extensions/src/nsExtensionManager.js.in;
/cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v  <--  nsExtensionManager.js.in
new revision: 1.253; previous revision: 1.252
done

Fixed, will pick up the test later. Thanks for spotting this Anthony.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: regression
OS: Linux → All
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M8
Attached patch testcase rev 1Splinter Review
This is a fairly simple testcase, installs 2 extensions both initially incompatible. The update.rdf should make one compatible and not the other. On restart check that one is installed and enabled and the other is not installed.

Minor wrinkle is the need for a timeout to wait for the compatibility check to complete, can't figure out a decent way to detect that the EM is done.
Attachment #280453 - Flags: review?(robert.bugzilla)
Litmus Triage Team: We may need to re-evaluate the need for this. robcee - what are the plans for automating this testing?
Perhaps a couple of new onStateChange values (INCOMPATIBLE_UPDATE_START and INCOMPATIBLE_UPDATE_DONE) could be added to nsIAddonUpdateListener and implement nsIAddonUpdateListener in the test? A hacky approach that should work:
for INCOMPATIBLE_UPDATE_START increment gInstallCount
for INCOMPATIBLE_UPDATE_DONE decrement gInstallCount
check gInstallCount == 0 after the switch
ignore DIALOG_CLOSE as we do DOWNLOAD_START, DOWNLOAD_DONE, and INSTALL_START in extensions.js

This would also solve the bug where the restart button is enabled before the incompatible update check is finished during normal operations as well as when installing updates during startup.

Thoughts?
Dave, I threw together a patch to implement what I suggested in comment #11... I'll file a bug and submit it later.
Cheers Rob, seems like a reasonable way to go
marcia: we don't have any current plans to test correct installation of add-ons yet. That might be a good candidate for a test suite for next quarter.
Filed Bug 396129 for the additional states for nsIAddonUpdateListener
Comment on attachment 280453 [details] [diff] [review]
testcase rev 1

Dropping the request till bug 396129 is resolved one way or another
Attachment #280453 - Flags: review?(robert.bugzilla)
Hi dave, this bug is marked RESOLVED FIXED, but i dont see the checkin yet.  When do you anticipate this landing?   Thanks.
(In reply to comment #17)
> Hi dave, this bug is marked RESOLVED FIXED, but i dont see the checkin yet. 
> When do you anticipate this landing?   Thanks.


https://bugzilla.mozilla.org/show_bug.cgi?id=395430#c8
Litmus triage team: tchung will handle this litmus testcase
Verified FIXED using:

Version 0.7.5.3 from September 24, 2007, downloaded from:

https://addons.mozilla.org/en-US/firefox/addon/1865

on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008011104 Minefield/3.0b3pre
Status: RESOLVED → VERIFIED
Product: Firefox → Toolkit
in-litmus flag cleanup.  
per comment 9, there are two tests already in-testsuite that covers this bug.
Flags: in-litmus? → in-litmus-
test_install.js verifies this scenario now
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.