Closed
Bug 723470
Opened 12 years ago
Closed 12 years ago
Add a method to cancel add-on installs in addons.js shared module
Categories
(Mozilla QA Graveyard :: Mozmill Tests, defect)
Mozilla QA Graveyard
Mozmill Tests
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: vladmaniac, Assigned: vladmaniac)
References
Details
(Whiteboard: [lib])
Attachments
(1 file, 2 obsolete files)
2.71 KB,
patch
|
whimboo
:
review+
|
Details | Diff | Splinter Review |
We need a method to cancel an add-on installation under addons.js shared module.
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → vlad.mozbugs
Status: NEW → ASSIGNED
Whiteboard: [mozmill-shared-modules]
Assignee | ||
Comment 1•12 years ago
|
||
See https://bugzilla.mozilla.org/show_bug.cgi?id=710143#c7 for further details
Assignee | ||
Comment 2•12 years ago
|
||
initial patch - included usage of the api function in testInstallTheme to prove helper function works - increased TIMEOUT constant to 7s instead of just 5s (in addonsManager.open()) Note: I was not able to trigger another modal dialog after cancel installation to say that the installation was cancelled. It just cancels it and the add-on is not being installed.
Attachment #593797 -
Flags: review?(hskupin)
Comment on attachment 593797 [details] [diff] [review] patch v1.0 - initial patch Just some preliminary feedback. I think you should deal with the test in it's own patch in it's own bug after the shared module patch has landed. Also, is the [mozmill-shared-module] whiteboard tag not redundant given that this bug is filed in the Mozmill Shared Module component?
Assignee | ||
Comment 4•12 years ago
|
||
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #3) > Comment on attachment 593797 [details] [diff] [review] > patch v1.0 - initial patch > > Just some preliminary feedback. I think you should deal with the test in > it's own patch in it's own bug after the shared module patch has landed. I see your point and it makes sense. But also, we need to prove that the shared module helper works, so I am using it in a mozmill test (kind of to replace a unit test for the api change). Let's also get Henrik's input on this IMHO > > Also, is the [mozmill-shared-module] whiteboard tag not redundant given that > this bug is filed in the Mozmill Shared Module component? Agreed - removed whiteboard tag
Whiteboard: [mozmill-shared-modules]
Comment 5•12 years ago
|
||
Comment on attachment 593797 [details] [diff] [review] patch v1.0 - initial patch >+ AddonManager.getAllInstalls(function (aInstalls) { >+ if (aInstalls.length) { >+ for (var i = 0; i < aInstalls.length; i++) { >+ aInstalls[i].cancel(); >+ } getAllInstalls always returns an array as given by the documentation on MDN. So there is no need to have a check for .length, which would even cause a strict js warning in this case. Please remove the if condition. >+exports.cancelExtensionInstalls = cancelExtensionInstalls; Not sure why this method name includes 'extension'. It applies to all types of add-ons. Please fix that. >+++ b/tests/functional/restartTests/testAddons_installTheme/test1.js Given the nature that this is toolkit code we do not have to test it. Just remove the test code from this patch and create a full patch for tests on the other bug.
Attachment #593797 -
Flags: review?(hskupin) → review-
Assignee | ||
Comment 6•12 years ago
|
||
Fixed
Attachment #593797 -
Attachment is obsolete: true
Attachment #594968 -
Flags: review?(hskupin)
Comment 7•12 years ago
|
||
Comment on attachment 594968 [details] [diff] [review] patch v1.1 >+function cancelAddonInstalls() { Sorry missed that one. Can we please give a better name for our own API? I would propose a name like 'cancelAddonInstallations'. With it fixed, r=me.
Attachment #594968 -
Flags: review?(hskupin) → review+
Assignee | ||
Comment 8•12 years ago
|
||
Nit fixed
Attachment #594968 -
Attachment is obsolete: true
Attachment #594991 -
Flags: review?(hskupin)
Comment 9•12 years ago
|
||
Comment on attachment 594991 [details] [diff] [review] fix patch v1.2 Looks good. Based on the broken tests I'm going to land this for all rapid release branches and mozilla-esr10.
Attachment #594991 -
Flags: review?(hskupin) → review+
Comment 10•12 years ago
|
||
Landed as: http://hg.mozilla.org/qa/mozmill-tests/rev/ce360cfa699c (default) http://hg.mozilla.org/qa/mozmill-tests/rev/3a7cc3324fd1 (aurora) http://hg.mozilla.org/qa/mozmill-tests/rev/3f179098a5ca (beta) http://hg.mozilla.org/qa/mozmill-tests/rev/149d8e373cc8 (release) http://hg.mozilla.org/qa/mozmill-tests/rev/2984713a45df (esr10)
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
OS: Linux → All
Hardware: x86 → All
Resolution: --- → FIXED
Updated•12 years ago
|
Component: Mozmill Shared Modules → Mozmill Tests
Updated•12 years ago
|
Whiteboard: [lib]
Updated•5 years ago
|
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•