The default bug view has changed. See this FAQ.

extension upgrade with a moved location breaks extension manager

VERIFIED FIXED in mozilla1.9.3a1

Status

()

Toolkit
Add-ons Manager
VERIFIED FIXED
8 years ago
6 years ago

People

(Reporter: Alexander Sack, Assigned: Alexander Sack)

Tracking

({fixed1.9.0.18})

1.9.0 Branch
mozilla1.9.3a1
x86
Linux
fixed1.9.0.18
Points:
---

Firefox Tracking Flags

(status1.9.2 beta5-fixed, status1.9.1 .8-fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

8 years ago
We moved our system langpacks from one location to another - previously not used one - for karmic; this seemed to have cause extension manager to go into a bad state (at least for me).

The symptoms are that during the upgrade, the uninstallItem function fails here:

http://hg.mozilla.org/mozilla-central/file/dbebe1771118/toolkit/mozapps/extensions/src/nsExtensionManager.js.in#l4591

with an exception like:

[Exception... "Could not convert JavaScript argument arg 1 [nsIRDFDataSource.GetTarget]"  nsresult: "0x80570009 (NS_ERROR_XPC_BAD_CONVERT_JS)"  location: "JS frame :: file:///usr/lib/xulrunner-1.9.1.5pre/components/nsExtensionManager.js :: EMDS__setProperty :: line 7250"  data: no]

... which breaks the whole extension update somewhat.

I looked a bit around thinking that maybe the UPGRADE op is wrong, but i think thats not the problem. It seems that the _setProperty code her:

http://hg.mozilla.org/mozilla-central/file/dbebe1771118/toolkit/mozapps/extensions/src/nsExtensionManager.js.in#l6939

is supposed to deal with this situation as it explicitly tests "if (oldValue)" ... however, GetTarget does not return null but throws and exception, so a potential fix would be to just catch it there.
(Assignee)

Comment 1

8 years ago
Created attachment 405845 [details] [diff] [review]
catch GetTarget exception
You don't assign oldValue, this ain't gonna work ;)
(Assignee)

Updated

8 years ago
Attachment #405845 - Flags: review?(dtownsend)
(Assignee)

Comment 3

8 years ago
Created attachment 405846 [details] [diff] [review]
oops
Attachment #405846 - Flags: review?(dtownsend)
(Assignee)

Updated

8 years ago
Attachment #405845 - Attachment is obsolete: true
Attachment #405845 - Flags: review?(dtownsend)
Assignee: nobody → asac
Attachment #405846 - Flags: review?(dtownsend) → review-
Comment on attachment 405846 [details] [diff] [review]
oops

GetTarget only throws when it is passed invalid arguments. We should fix the caller to not do that instead.
(Assignee)

Comment 5

8 years ago
(In reply to comment #4)
> (From update of attachment 405846 [details] [diff] [review])
> GetTarget only throws when it is passed invalid arguments. We should fix the
> caller to not do that instead.

invalid in what way? I always thought that GetTarget also throws if "source" resource does not exist for example.
GetTarget only throws if source or property are null, however the error message you're getting isn't from GetTarget throwing, it is xpconnect throwing because source or property are not null or JS objects that it can pretend are nsIRDFResources.

Comment 7

8 years ago
Ubuntu bug:
https://bugs.launchpad.net/bugs/441552
(Assignee)

Comment 8

8 years ago
Created attachment 407108 [details] [diff] [review]
fix type of newVersion property...

ok newVersion was just a string while setItemProperty/setProperty requires a nsIRDFResource ... this patch fixes it by using EM_R("newVersion")

That said I noticed that setItemProperty assumes a nsIRDFResource as "property", while getItemProperty seems to EM_R "property" all the time. Might be worth looking into making the behaviour more consistent.
Attachment #405846 - Attachment is obsolete: true
Attachment #407108 - Flags: review?(dtownsend)
Attachment #407108 - Flags: review?(dtownsend) → review+
http://hg.mozilla.org/mozilla-central/rev/a1749e78c2be
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Attachment #407108 - Flags: approval1.9.2?
Attachment #407108 - Flags: approval1.9.1.7?
Comment on attachment 407108 [details] [diff] [review]
fix type of newVersion property...

Ubuntu is already using this patch on their 1.9.1 branch, so would like to get this backported.
Attachment #407108 - Flags: approval1.9.1.6?
Attachment #407108 - Flags: approval1.9.0.17?
Attachment #407108 - Flags: approval1.9.0.16?
Attachment #407108 - Flags: approval1.9.1.6?
Attachment #407108 - Flags: approval1.9.0.16?
Comment on attachment 407108 [details] [diff] [review]
fix type of newVersion property...

a192=beltzner
Attachment #407108 - Flags: approval1.9.2? → approval1.9.2+
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/760b6fefebcc
status1.9.2: --- → final-fixed
Comment on attachment 407108 [details] [diff] [review]
fix type of newVersion property...

Approved for 1.9.1.7 and 1.9.0.17, a=dveditz for release-drivers
Attachment #407108 - Flags: approval1.9.1.7?
Attachment #407108 - Flags: approval1.9.1.7+
Attachment #407108 - Flags: approval1.9.0.17?
Attachment #407108 - Flags: approval1.9.0.17+
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/7b12d53eed85

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.290; previous revision: 1.289
done
status1.9.1: --- → .7-fixed
Keywords: fixed1.9.0.17

Updated

7 years ago
The nsExtensionManager.js.in module is gone on trunk. Just marking it as verified fixed.
Status: RESOLVED → VERIFIED
Version: unspecified → 1.9.0 Branch
You need to log in before you can comment on or make changes to this bug.