Closed
Bug 993539
Opened 11 years ago
Closed 11 years ago
sourceURI property of add-on fails if the repository has been updated
Categories
(Toolkit :: Add-ons Manager, defect)
Toolkit
Add-ons Manager
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: mossop, Assigned: mossop)
Details
Attachments
(1 file, 1 obsolete file)
4.70 KB,
patch
|
Irving
:
review+
|
Details | Diff | Splinter Review |
Because we attempt to create a new nsIURI from an nsIURI :s
Attachment #8403452 -
Flags: review?(irving)
Comment 1•11 years ago
|
||
Comment on attachment 8403452 [details] [diff] [review]
patch
Review of attachment 8403452 [details] [diff] [review]:
-----------------------------------------------------------------
This looks somewhat broken in the surrounding context; other places in XPIProvider and XPIProviderUtils assume that AddonInternal.releaseNotesURI is a string - we try to put it directly into the JSON to save, and in AI_updateAddonURIs we deliberately assign AddonInstall.releaseNotesURI.spec to AddonInternal.releaseNotesURI.
I think the right fix is to ensure we always store the string (nsIURI.spec) in AddonInternal.releaseNotesURI.
In other news, I seem to be drunk with my newfound powers.
Attachment #8403452 -
Flags: review?(irving) → review-
Assignee | ||
Comment 2•11 years ago
|
||
(In reply to :Irving Reid from comment #1)
> Comment on attachment 8403452 [details] [diff] [review]
> patch
>
> Review of attachment 8403452 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> This looks somewhat broken in the surrounding context; other places in
> XPIProvider and XPIProviderUtils assume that AddonInternal.releaseNotesURI
> is a string - we try to put it directly into the JSON to save, and in
> AI_updateAddonURIs we deliberately assign AddonInstall.releaseNotesURI.spec
> to AddonInternal.releaseNotesURI.
>
> I think the right fix is to ensure we always store the string (nsIURI.spec)
> in AddonInternal.releaseNotesURI.
That's right, AddonInternal.releaseNotesURI and AddonInternal.sourceURI are all strings. This code is generating the value of AddonWrapper.sourceURI aand AddonWrapper.releaseNotesURI though which are all nsIURIs. The problem is that AddonInternal.sourceURI is a string and the cached repository add-on's sourceURI is an nsIURI (http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/internal/AddonRepository.jsm#1828). Right now the code assumes it is a string for some reason (maybe it used to be?) so it tries to generate a new nsIURI from it which fails.
Flags: needinfo?(irving)
Comment 3•11 years ago
|
||
Comment on attachment 8403452 [details] [diff] [review]
patch
Review of attachment 8403452 [details] [diff] [review]:
-----------------------------------------------------------------
::: toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ +6326,5 @@
> let target = chooseValue(aAddon, aProp)[0];
> if (!target)
> return null;
> + if (target instanceof Ci.nsIURI)
> + return target;
OK, I've read through this code enough times now. Aside from conflicting feelings about the confusing mix of representations between our object types, and wanting a test, how about:
let [target, fromRepo] = chooseValue(aAddon, aProp);
if (!target)
return null;
if (fromRepo)
return target;
return NetUtil.newURI(target);
Updated•11 years ago
|
Flags: needinfo?(irving)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → dtownsend+bugmail
Assignee | ||
Comment 4•11 years ago
|
||
Yeah the mixed types sucks but right now I want to get this fixed quickly as it's breaking the add-on debugger :(
Attachment #8403452 -
Attachment is obsolete: true
Attachment #8404278 -
Flags: review?(irving)
Updated•11 years ago
|
Attachment #8404278 -
Flags: review?(irving) → review+
Assignee | ||
Comment 5•11 years ago
|
||
Comment 6•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•