Progress bar appears under wrong add-on when installing from search list

VERIFIED FIXED

Status

Firefox for Android Graveyard
General
VERIFIED FIXED
8 years ago
7 years ago

People

(Reporter: mbrubeck, Assigned: wesj)

Tracking

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

8 years ago
Steps to reproduce:
1. Open the add-on panel.
2. Do a search that returns more than one result.
3. Tap an add-on that is *not* the first result, and press "Add to Fennec."

Expected results: Progress bar appears under the selected add-on and shows installation progress.

Actual results: One progress bar appears under the selected add-on and stays empty; a second progress bar appears under the first add-on in the list and shows progress.  (But after restart, the selected add-on is installed.)
Assignee: mbrubeck → wjohnston
(Assignee)

Comment 1

8 years ago
Created attachment 467203 [details] [diff] [review]
Use sourceURI.spec... sometimes

This bug was fallout from bug 575463, which changed sourceURL to sourceURI. I'm not really sure how it was working before though.

When we originally create the items we have access to the extension id (myExtension@me.org). We used that as the ID of the richlistitems. When the download progress events fire, we don't have access to the id (fallout from 575463?), just the URL we're downloading from, so we can't find the items.

This patch changes the manager to use the source url for the richlistitem if we have an install object. Otherwise, it uses the extension id. Not sure if that's the perfect solution. Glad to hear if there's a better way.
Attachment #467203 - Flags: review?(mark.finkle)
Comment on attachment 467203 [details] [diff] [review]
Use sourceURI.spec... sometimes

 
>   _createItem: function ev__createItem(aAddon, aTypeName) {
>     let item = document.createElement("richlistitem");
>-    item.setAttribute("id", PREFIX_ITEM_URI + aAddon.id);
>+    let itemID = PREFIX_ITEM_URI;
>+    if(aAddon.install)
>+      itemID += aAddon.install.sourceURI.spec;
>+    else
>+      itemID += aAddon.id;
>+    item.setAttribute("id", itemID);

I think we don't need to change this

sourceURL is set when we list addons in a search results. See:
http://mxr.mozilla.org/mobile-browser/source/chrome/content/extensions.js#516

"addon.install.sourceURL" should probably be "addon.install.sourceURI.spec"

getElementForAddon will look in the "sourceURL" is needed:
http://mxr.mozilla.org/mobile-browser/source/chrome/content/extensions.js#118
Attachment #467203 - Flags: review?(mark.finkle) → review-
(Assignee)

Comment 3

8 years ago
Created attachment 467273 [details] [diff] [review]
replace sourceURL

Whoops. Thanks.
Attachment #467203 - Attachment is obsolete: true
Attachment #467273 - Flags: review?(mark.finkle)
Attachment #467273 - Flags: review?(mark.finkle) → review+
Whiteboard: [fennec-checkin-posta1]
pushed:
http://hg.mozilla.org/mobile-browser/rev/5c6669248f13
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Whiteboard: [fennec-checkin-posta1]
This issue is verified as fixed on Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110919 Firefox/9.0a1 Fennec/9.0a1
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.