Closed Bug 982295 Opened 6 years ago Closed 5 years ago

[Gaia][Browser] Long Press Downloads for Media should route downloads to Downloads API

Categories

(Firefox OS Graveyard :: Gaia::Browser, defect)

x86
macOS
defect
Not set

Tracking

(blocking-b2g:-, feature-b2g:2.0, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
2.0 S4 (20june)
blocking-b2g -
feature-b2g 2.0
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: aus, Assigned: aus)

References

Details

(Whiteboard: [systemsfe][p=5])

Attachments

(3 files, 1 obsolete file)

Currently the Browser manually manages downloading Media assets (images, audio and video). It should route these to the Downloads API using an anchor tag with download and type attributes set accordingly.

Example:

<a href="http://hostname/path/to/media.mp3" download="media.mp3" type="application/octet-stream"></a>

This element can then be "clicked" or "pressed" to trigger a download that will be completely managed by the Downloads API (and Gaia's Download Manager).
Whiteboard: [systemsfe]
blocking-b2g: --- → 1.4?
Blocks: 929532
I think this is a good idea, but I don't think we would hold a release on this, mainly because it's cleanup work for consistency with the new download manager. Feel free to move forward implementing this though.
blocking-b2g: 1.4? → backlog
My work in progress patch seems to show that an <a href='...' download='...' type='application/octet-stream'> added to the browser app does not trigger a download when clicked.

The patch adds an anchor tag to the DOM programmatically, but if I just add an anchor tag to index.html of the browser app manually I see the same result. Clicking on the link just navigates the app window to the URL rather than triggering a download.

Is this a bug, or am I missing something?
Assignee: nobody → bfrancis
Flags: needinfo?(fabrice)
Flags: needinfo?(aus)
Ben, going to try and look at this tomorrow... sorry about the wait. Leaving NI? flag untouched as a reminder :)
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a states that "In Firefox 20 this attribute is only honored for links to resources with the same-origin". Not sure if this should be read as "Starting in Firefox 20..." which would explain why it's not working there.
Flags: needinfo?(fabrice)
(In reply to Fabrice Desré [:fabrice] from comment #4)
> https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a states that "In
> Firefox 20 this attribute is only honored for links to resources with the
> same-origin". Not sure if this should be read as "Starting in Firefox 20..."
> which would explain why it's not working there.

Yep, that's it.

I read this, but as Firefox 20 was the first version to support the download attribute I concluded it meant that Firefox 20 didn't support cross-origin downloads but future versions do.

I just created a test case which shows this isn't the case http://people.mozilla.org/~bfrancis/download.html

A cross-origin <a download> link will never trigger a download in Gecko.

That means if we want users to be able to manually trigger a download of a file from an image/audio/video element or hyperlink we will either need to lift this restriction or create a new method in the download API for that. Otherwise I guess we have to add the download API permission to the browser app.
(In reply to Ben Francis [:benfrancis] from comment #5)
> Otherwise I guess we have to add the download API permission to the browser
> app.

Actually I'm not sure that would even help, is there even a privileged API call that can manually trigger a download from an app?
(In reply to Ben Francis [:benfrancis] from comment #6)
> (In reply to Ben Francis [:benfrancis] from comment #5)
> > Otherwise I guess we have to add the download API permission to the browser
> > app.
> 
> Actually I'm not sure that would even help, is there even a privileged API
> call that can manually trigger a download from an app?

Damn, that sucks. And yep, we'd have to add a method to trigger a download from a URI. That doesn't exist right now. It shouldn't be too difficult to do this and it does sound like it would be awfully useful for applications to have a way to do this. I'm going to go ahead and file a bug to add this method to the Downloads API and add it as a dependent of this bug.
Flags: needinfo?(aus)
Depends on: 983747
Unassigning myself for now, until we figure out platform support for this and the path forward.
Assignee: bfrancis → nobody
Assignee: nobody → aus
Status: NEW → ASSIGNED
Target Milestone: --- → 2.0 S1 (9may)
Blocks: 906262
feature-b2g: --- → 2.0
Target Milestone: 2.0 S1 (9may) → 2.0 S2 (23may)
Whiteboard: [systemsfe] → [systemsfe][p=5]
Attachment #8419673 - Attachment is obsolete: true
Attachment #8423494 - Flags: review?(bfrancis)
Comment on attachment 8423494 [details] [review]
Pull Request - Use new 'download' method on Browser API.

This is awesome.
Attachment #8423494 - Flags: review?(bfrancis) → review+
(In reply to Ben Francis [:benfrancis] from comment #11)
> Comment on attachment 8423494 [details] [review]
> Pull Request - Use new 'download' method on Browser API.
> 
> This is awesome.

I love replacing a big pile of code with one line. It's so good. So good! :D
Duplicate of this bug: 1014774
Target Milestone: 2.0 S2 (23may) → 2.0 S3 (6june)
No longer blocks: 929532
Blocks: 876376
Rob is PTO today so I'm posting this on his behalf. He noted that this prompt doesn't have the right style. I'll post him as needs info to follow up.
Flags: needinfo?(rmacdonald)
Example of value selector from 2.0 (from Email).
> Created attachment 8434367 [details]
> Value Selector Example (Email)
> 
> Example of value selector from 2.0 (from Email).

The attached example shows the intended format of the value selector in 2.0. Also flagging Eric in case he has anything to add.
Flags: needinfo?(rmacdonald) → needinfo?(epang)
Nothing to add, this looks good to me :). Thanks guys!
Flags: needinfo?(epang)
NI aus, to get this landed before the merge tomorrow, as this looks ready..
Flags: needinfo?(aus)
(In reply to bhavana bajaj [:bajaj] from comment #18)
> NI aus, to get this landed before the merge tomorrow, as this looks ready..

There are dependencies which are clearly listed that have not landed. Please read the whole bug before flagging for needinfo.

(In reply to Rob MacDonald [:robmac] from comment #16)
> > Created attachment 8434367 [details]
> > Value Selector Example (Email)
> > 
> > Example of value selector from 2.0 (from Email).
> 
> The attached example shows the intended format of the value selector in 2.0.
> Also flagging Eric in case he has anything to add.

Hi guys, this bug is purely to update which API the downloads use. If we want to change and update the visual we'll want to file a new bug.
Flags: needinfo?(aus)
Requesting approval for 2.0 landing now since this requires a Gecko patch that will take a day or two to show up in Aurora.
blocking-b2g: backlog → 2.0?
(In reply to Ghislain Aus Lacroix [:aus] from comment #20)
> Requesting approval for 2.0 landing now since this requires a Gecko patch
> that will take a day or two to show up in Aurora.

(In reply to Ghislain Aus Lacroix [:aus] from comment #19)
> (In reply to bhavana bajaj [:bajaj] from comment #18)
> > NI aus, to get this landed before the merge tomorrow, as this looks ready..
> 
> There are dependencies which are clearly listed that have not landed. Please
> read the whole bug before flagging for needinfo.

I did check the dependency, https://bugzilla.mozilla.org/show_bug.cgi?id=983747#c34 and it looked almost ready to land hence flagged you here.
> 
> (In reply to Rob MacDonald [:robmac] from comment #16)
> > > Created attachment 8434367 [details]
> > > Value Selector Example (Email)
> > > 
> > > Example of value selector from 2.0 (from Email).
> > 
> > The attached example shows the intended format of the value selector in 2.0.
> > Also flagging Eric in case he has anything to add.
> 
> Hi guys, this bug is purely to update which API the downloads use. If we
> want to change and update the visual we'll want to file a new bug.

I don't think we would block on this bug for a release, as comment #1 explains. If you are ready to uplift this and the dependency in the next few days, please seek gaia/gecko approvals. Based on the risk/reward of this patch, the approval will be acted upon.
blocking-b2g: 2.0? → -
Target Milestone: 2.0 S3 (6june) → 2.0 S4 (20june)
> I don't think we would block on this bug for a release, as comment #1
> explains. If you are ready to uplift this and the dependency in the next few
> days, please seek gaia/gecko approvals. Based on the risk/reward of this
> patch, the approval will be acted upon.

I'll go ahead and ask for approval on this item once the patch for bug 983747 makes it into Aurora. Apologies, status in bug 983747 could have been clearer.
Comment on attachment 8423494 [details] [review]
Pull Request - Use new 'download' method on Browser API.

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 876376
[User impact] if declined: Functionality as present is broken under many scenarios.
[Testing completed]: On device with custom gecko using patch from bug 983747 and gaia master + this pull request. 
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: none
Attachment #8423494 - Flags: approval-gaia-v2.0?
Why is this feature absolutely needed in 2.0?
Commit (master): https://github.com/mozilla-b2g/gaia/commit/901646a3279c66daa7621bebb62641779d8ae22c

Fixed on master. I'm leaving this open until it 983747 lands on aurora.
Keywords: leave-open
Attachment #8423494 - Flags: approval-gaia-v2.0? → approval-gaia-v2.0+
(In reply to Jason Smith [:jsmith] from comment #24)
> Why is this feature absolutely needed in 2.0?

This missed the 2.0 train due to a gecko uplift by a few hours and merge day activity and this exception was discussed with Release management (:bajaj) to get this uplifted keeping the risk/reward in mind.
Commit (v2.0): https://github.com/mozilla-b2g/gaia/commit/593b6b1765c76017fa575066f1139e5332595804

Fixed.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: leave-open
Resolution: --- → FIXED
No longer depends on: 1025853
Depends on: 1034828
You need to log in before you can comment on or make changes to this bug.