Closed Bug 1305663 Opened 8 years ago Closed 5 years ago

Support downloadItem.byExtensionId

Categories

(WebExtensions :: General, defect, P3)

Product:
WebExtensions
Component:
General
Version:
51 Branch
Type:
defect

Tracking

(firefox69 verified)

Status:
VERIFIED FIXED
Milestone:
mozilla69
Tracking Flags:
Tracking Status
firefox69 --- verified

People

(Reporter: mostafa.elsaie, Assigned: mozbugzilla)

References

(Blocks 1 open bug)

Details

(Whiteboard: triaged[downloads])

Attachments

(4 files, 2 obsolete files)

downloads-byExtensionId.zip
(deleted), application/zip
Details
Bug 1305663 - add test for download's extension properties
(deleted), text/x-phabricator-request
Details
Bug 1305663 - save extension info to DownloadMap properly.
(deleted), text/x-phabricator-request
Details
Bug 1305663 - save extension info to DownloadMap properly, r=aswan
(deleted), text/x-phabricator-request
Details
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.79 Safari/537.36 Edge/14.14393 Steps to reproduce: 1) Initiate a download from an extension using the downloads.download(). 2) Try to analyse the dowloadItem in the downloads.onCreated event. 3) Look for the byExtensionId & byExtensionName attributes of the downloadItem. Actual results: Both attributes are always "undefined" Expected results: Both attributes should contain the extensions ID & Name respectively.
OS: Unspecified → Windows 10
Hardware: Unspecified → x86_64
Component: WebExtensions: Untriaged → WebExtensions: General
OS: Windows 10 → All
Hardware: x86_64 → All
Summary: downloadItem.byExtensionId always returns undefined → Support downloadItem.byExtensionId
Priority: -- → P3
Whiteboard: triaged
Whiteboard: triaged → triaged[downloads]
Attached file downloads-byExtensionId.zip (deleted) —
Confirmed in Firefox 55.0b5. 1. Install the attached add-on (via about:debugging for example). 2. Click on the extension button in the toolbar that appears after installing the extension. 3. Open the global browser console and look at the printed messages. Result: onCreated at 7/9/2017, 10:42:31 PM background.js:2:5 id = 8 background.js:4:9 ... (cut) ... byExtensionId = undefined background.js:4:9 byExtensionName = undefined background.js:4:9 downloads.download callback with ID: 8 background.js:12:9 Note hat byExtensionId and byExtensionName are both undefined, while they should have a sensible value. The only place where the extension is set for a DownloadItem is for the callback of downloads.download [1]. However, the downloads.onCreated event fires before that [2] and causes a DownloadItem to be created without extension. The result is cached too, so when the download.download callback is invoked, the DownloadItem is not cached (nor updated with the extension). Even if the above bug were to be fixed, the whole implementation would only work for downloads created in the current session; if the browser is restarted, the byExtensionId/byExtensionName fields would be cleared. Though if the browser is restarted, the downloads list would be empty anyway, because of bug 1255507. And, if an extension is uninstalled, the byExtensionId and byExtensionName fields are cleared too (contrary to Chrome, which shows the ID and the last known name if the extension has been uninstalled). [1] https://searchfox.org/mozilla-central/rev/4aaa08513911c3cfe750611a6c087b438bb1f88e/toolkit/components/extensions/ext-downloads.js#551 [2] https://searchfox.org/mozilla-central/rev/4aaa08513911c3cfe750611a6c087b438bb1f88e/toolkit/components/extensions/ext-downloads.js#163-165
(updating bug flags based on previous comment)
Status: UNCONFIRMED → NEW
Ever confirmed: true
See Also: → https://bugzilla.mozilla.org/show_bug.cgi?id=1255507
This is an interoperability issue too, so blocking bug 1213445.
Blocks: 1213445
Product: Toolkit → WebExtensions
I'm working on an WebExt and I'm getting both undefined for byExtensionId and byExtensionName. Is it normal that it's marked as supported on MDN Browser compatibility table ? https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/downloads/DownloadItem#Browser_compatibility
Attached patch add test for download's extension properties (obsolete) (deleted) — — Splinter Review
Attached patch save extension info to DownloadMap properly (obsolete) (deleted) — — Splinter Review

Hi Martin, thanks for your patch! It looks very clean.

Could you upload it to our Phabricator (documentation here), so that it can be reviewed and landed?

If you are not able to (e.g. because you cannot add a 2-factor method to your Bugzilla account, which is required to log in to Phabricator), then the review can also be done here. This is a bit inconvenient, but better than not taking the patches at all.

Flags: needinfo?(matous)

Previous implementation created new DownloadItem with a null as an indirect result of list.add()

Depends on D30591

(In reply to Rob Wu [:robwu] from comment #7)

Hi Martin, thanks for your patch! It looks very clean.

Could you upload it to our Phabricator (documentation here), so that it can be reviewed and landed?

If you are not able to (e.g. because you cannot add a 2-factor method to your Bugzilla account, which is required to log in to Phabricator), then the review can also be done here. This is a bit inconvenient, but better than not taking the patches at all.

Done. I did not feel like reading yet more documentation and configuring yet more tools that day, but it turned to be pretty easy.

Attachment #9063300 - Attachment is obsolete: true
Flags: needinfo?(matous)
Attachment #9063302 - Attachment is obsolete: true

Previous implementation created new DownloadItem with a null as an indirect result of list.add()

Assignee: nobody → matous-dev
Pushed by aswan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9850cc6f4b7a save extension info to DownloadMap properly, r=aswan

https://hg.mozilla.org/mozilla-central/rev/9850cc6f4b7a

Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox69: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

Thanks so much for the patch, Martin! 🎉 Your contribution has been added to our recognition wiki: https://wiki.mozilla.org/Add-ons/Contribute/Recognition

Would you be interested in creating an account on mozillians.org? I'd be happy to vouch for you!

Thanks. I created an account. But... reCAPTCHA? Really? I expected better.

Verified as fixed using the steps from comment 1 using FF69.0b3 and FF70.0a1(20190708182549) in Win7x64 and Ubuntu 14.04

status-firefox69: fixed → verified
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: