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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: mossop, Assigned: mossop)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
Because we attempt to create a new nsIURI from an nsIURI :s
Attachment #8403452 - Flags: review?(irving)
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-
(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 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);
Flags: needinfo?(irving)
Assignee: nobody → dtownsend+bugmail
Attached patch patchSplinter Review
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)
Attachment #8404278 - Flags: review?(irving) → review+
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.

Attachment

General

Created:
Updated:
Size: