Closed
Bug 314915
Opened 19 years ago
Closed 18 years ago
Extension update can identify the wrong version as being the newest.
Categories
(Toolkit :: Add-ons Manager, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha7
People
(Reporter: mossop, Assigned: mossop)
References
Details
Attachments
(3 files, 2 obsolete files)
The update for extension seems to make assumptions about the ordering of the update rdf for an extension. It looks like it generally assumes that the last version listed in the file is the newest version.
As an example I will attach an empty extension and the update rdf that it points to. If you install the extension and then attempt to update it firefox will claim to have found a newer version, version 3. However looking at the update file it is clear that there is a version 4 available.
From testing with the real extension that this problem was seen with I know that was the exctension actually version 3 then it would correctly find version 4 in that case.
Also, moving the update identifier for version 4 at the top of the file to be after version 3 means that version 4 is always identified as the latest version.
Assignee | ||
Comment 1•19 years ago
|
||
Test extension
Assignee | ||
Comment 2•19 years ago
|
||
The is a copy of the update rdf that the test extension updates from.
Comment 3•19 years ago
|
||
This should probably be done in the next re-write - I could have sworn the docs mentioned that the orderring was significant but can't find any that do so.
Assignee | ||
Comment 4•19 years ago
|
||
Having to maintain an order in the update rdf is somewhat of a pain for people like me who (foolishly) have a semi-automated update system. It means basically implementing the version checker logic and so of course is susceptible to changes and bugs in it.
Updated•19 years ago
|
OS: Windows XP → All
Hardware: PC → All
Assignee | ||
Comment 5•18 years ago
|
||
This changes the parsing loop to check whether each potential update is a valid update when the update version is the same as the current version (in version update only case) or the update version is larger than the last largest seen version (which is initilised to the current version).
Assignee: nobody → dtownsend
Status: NEW → ASSIGNED
Attachment #267607 -
Flags: review?(robert.bugzilla)
Comment 6•18 years ago
|
||
Comment on attachment 267607 [details] [diff] [review]
patch rev 1
Dave, this looks good. Can you fix all of the tabs in the patch, update it for the recent changes on trunk from bug 335942, and re-request? Thanks!
Attachment #267607 -
Flags: review?(robert.bugzilla) → review-
Assignee | ||
Comment 7•18 years ago
|
||
Updated to trunk.
Attachment #267607 -
Attachment is obsolete: true
Attachment #270429 -
Flags: review?(robert.bugzilla)
Comment 8•18 years ago
|
||
Comment on attachment 270429 [details] [diff] [review]
patch rev 2
>Index: toolkit/mozapps/extensions/src/nsExtensionManager.js.in
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v
>retrieving revision 1.223
>diff -u8 -p -r1.223 nsExtensionManager.js.in
>--- toolkit/mozapps/extensions/src/nsExtensionManager.js.in 25 Jun 2007 21:38:19 -0000 1.223
>+++ toolkit/mozapps/extensions/src/nsExtensionManager.js.in 30 Jun 2007 16:25:25 -0000
>@@ -6036,30 +6036,30 @@ ExtensionItemUpdater.prototype = {
> return true;
> }
> else if (this._updateCheckType == nsIExtensionManager.UPDATE_SYNC_COMPATIBILITY)
> this._emDS.updateTargetAppInfo(aLocalItem.id, aRemoteItem.minAppVersion,
> aRemoteItem.maxAppVersion);
> return false;
> },
>
>- _isValidUpdate: function(aLocalItem, aRemoteItem) {
>+ _isValidUpdate: function(aLocalItem, aVersion, aMinAppVersion, aMaxAppVersion) {
nit: it isn't clear what aVersion, aMinAppVersion, and aMaxAppVersion are from now that they aren't associated with aRemoteItem. Either comment the function or use param names that describe what they are.
>@@ -6477,52 +6470,55 @@ RDFItemUpdater.prototype = {
>...
> _parseV20Update: function(aDataSource, aUpdateResource, aLocalItem, aUpdateData, aUpdateCheckType) {
> var version = this._getPropertyFromResource(aDataSource, aUpdateResource,
> "version", aLocalItem);
> var taArc = gRDF.GetResource(EM_NS("targetApplication"));
> var targetApps = aDataSource.GetTargets(aUpdateResource, taArc, true);
> while (targetApps.hasMoreElements()) {
> var targetApp = targetApps.getNext().QueryInterface(Components.interfaces.nsIRDFResource);
> var id = this._getPropertyFromResource(aDataSource, targetApp, "id", aLocalItem);
> if (id != this._updater._appID)
> continue;
>
>- var result = gVersionChecker.compare(version, aLocalItem.version);
>+ var result = gVersionChecker.compare(version, aUpdateData.version);
> if (aUpdateCheckType == nsIExtensionManager.UPDATE_CHECK_NEWVERSION ? result > 0 : result == 0) {
nit: a comment describing this if statement would be a good thing.
With those fixed r=me. Thanks!
Attachment #270429 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 9•18 years ago
|
||
Added those comments, carrying over review, ready for checkin.
Attachment #270429 -
Attachment is obsolete: true
Attachment #271122 -
Flags: review+
Assignee | ||
Comment 10•18 years ago
|
||
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.226; previous revision: 1.225
done
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 beta1
Comment 11•18 years ago
|
||
Mossop, can we talk? I have to undo half the change rstrong noted in comment 8 part 1 for bug 299716, because I have to check ID as well. It's better in this case to pass aRemoteItem, so I can check that too.
Updated•17 years ago
|
Flags: in-testsuite?
Assignee | ||
Comment 12•17 years ago
|
||
This is tested by the testcase for bug 394300
Flags: in-testsuite? → in-testsuite+
Comment 13•17 years ago
|
||
Dave, do you have an updated testcase you can attach that is supported for the trunk? i want to confirm the fix.
The test extension attached in comment 1 complains about not working for: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008011104 Minefield/3.0b3pre
Updated•16 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•