Closed Bug 983747 Opened 6 years ago Closed 5 years ago

[Browser API] Add a method to download a file

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
2.0 S4 (20june)
feature-b2g 2.0
tracking-b2g backlog
Tracking Status
firefox31 --- wontfix
firefox32 --- fixed
firefox33 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: aus, Assigned: aus)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [systemsfe][p=8])

Attachments

(2 files, 4 obsolete files)

Currently the Downloads API does not provide a way for a certified application to trigger a download directly. We should have one for convenience so that applications can offload downloads to the download manager.
blocking-b2g: 1.4? → backlog
Assignee: nobody → aus
Target Milestone: --- → 2.0 S1 (9may)
Working on this starting today.
Status: NEW → ASSIGNED
Attachment #8418457 - Flags: review?(fabrice)
Comment on attachment 8418457 [details] [diff] [review]
Patch - v1 - Add 'download' method to Downloads API

Review of attachment 8418457 [details] [diff] [review]:
-----------------------------------------------------------------

I haven't reviewed the test yet, but I have a general question: I guess on use case is to trigger downloads for use cases like "Save image" on a long tap. In this situation, how do we deal with cookies that are set on the page but are lost with this api? It's likely to make the download fail. Same issue with http auth if any.

::: dom/downloads/src/DownloadsAPI.js
@@ +53,5 @@
>    },
>  
> +  download: function(aURL, aOptions) {
> +    debug("download(aURL = " + aURL + ", aOptions = " + aOptions + ")");
> +    

nit: whitespace.

::: dom/downloads/src/DownloadsAPI.jsm
@@ +187,5 @@
> +      let downloadPath = aData.options.targetDirectory;
> +
> +      if (!downloadFilename) {
> +        let url = new URL(aData.url);
> +        downloadFilename = url.pathname.split('/').pop();

what happens if I donwload http://foo.com/ ?

@@ +194,5 @@
> +      if (!downloadPath) {
> +        downloadPath = defaultDir;
> +      }
> +
> +      debug('creating download > ');

here and for other strings: we use double quotes in this file.

@@ +224,5 @@
> +      let list = yield Downloads.getList(Downloads.ALL);
> +      // Add our download to it.
> +      list.add(download);
> +    }).then(null, Cu.reportError);
> +    

nit: whitespace.

::: dom/downloads/src/DownloadsIPC.jsm
@@ +161,5 @@
> +    debug("download(aURL = " + aURL + ", aOptions = " + aOptions + ")");
> +    let deferred = Promise.defer();
> +    let pId = this.promiseId();
> +    this.downloadPromises[pId] = deferred;
> +    cpmm.sendSyncMessage("Downloads:Download", 

nit: whitespace.

::: dom/webidl/Downloads.webidl
@@ +73,5 @@
>  
>    // An opaque identifier for this download. All instances of the same
>    // download (eg. in different windows) will have the same id.
>    readonly attribute DOMString id;
> +  

nit: whitespace.

@@ +99,5 @@
> +  // Download requested via private browsing session?
> +  boolean isPrivate = false;
> +  // HTTP Referrer to send to server.
> +  DOMString? referrer = null;
> +  // Specify target filename

nit: full stop at the end of comment.

@@ +101,5 @@
> +  // HTTP Referrer to send to server.
> +  DOMString? referrer = null;
> +  // Specify target filename
> +  DOMString? targetFilename  = null;
> +  // Specify target directory (from primary writable storage root)

ditto
Attachment #8418457 - Flags: review?(fabrice) → feedback+
(In reply to Fabrice Desré [:fabrice] from comment #3)
> Comment on attachment 8418457 [details] [diff] [review]
> Patch - v1 - Add 'download' method to Downloads API
> 
> Review of attachment 8418457 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I haven't reviewed the test yet, but I have a general question: I guess on
> use case is to trigger downloads for use cases like "Save image" on a long
> tap. In this situation, how do we deal with cookies that are set on the page
> but are lost with this api? It's likely to make the download fail. Same
> issue with http auth if any.

Indeed. The way it's implemented currently, it would also fail since we're using a separate XHR to download the content and we initiate this from a different context than the browser window which has said content.

I would love for us to support these use cases (1) Cookies Required and (2) Auth Required.

That being said, I think sites which have content for user download will have a way to trigger this action on the page which will route the download in such a way that cookies and previous authentication are correctly used.
 
> ::: dom/downloads/src/DownloadsAPI.jsm
> @@ +187,5 @@
> > +      let downloadPath = aData.options.targetDirectory;
> > +
> > +      if (!downloadFilename) {
> > +        let url = new URL(aData.url);
> > +        downloadFilename = url.pathname.split('/').pop();
> 
> what happens if I donwload http://foo.com/ ?

I think it's probably best to error out if someone passes in a URL with no leaf name. I'll add a check for this.
 
I'll address the other nits as well. :)
Ehsan, Jonas, we're looking for your opinions on this new methods abilities.

See https://bugzilla.mozilla.org/show_bug.cgi?id=983747#c3 and https://bugzilla.mozilla.org/show_bug.cgi?id=983747#c4
Flags: needinfo?(jonas)
Flags: needinfo?(ehsan)
feature-b2g: --- → 2.0
It's not clear to me what the use case for this API is exactly.  If it's the certified app just wanting to initiate a download on its own, why can't we use <a @download> for example?  If it's the certified app wanting to initiate a download on behalf of other pages (such as a web page loaded in a mozbrowser) then this is not the right way to do things because of the cookie and HTTP auth issue that Fabrice raised, and the issue is actually worse than not having the right cookie, the request will have the cookies of the certified app, which may leak unintended information to the server.  Can you please clarify what the use case is?  Note that Necko receives the appId for the app which initiates a network request, and that is how it decides which cookie jar to look at.

Also, looking at the patch, the isPrivate flag caught my attention as well.  What is that flag supposed to do?  Firefox OS doesn't currently support private browsing, and if we want to allow private browsing downloads, this is probably insufficient on its own.  What is the use case for private downloads without support for private browsing?
Flags: needinfo?(ehsan) → needinfo?(aus)
The specific case we want to cover here is the ability for apps to formally request a download via an api that enables specifying where and how the file is named. In the case of the browser app, it needs this to allow downloading content arbitrarily without the web page providing a way to do this. It also makes the download route through the fxos download manager so that the user can manage it.

Unfortunately, we cannot use the <a @download> trick. See https://bugzilla.mozilla.org/show_bug.cgi?id=982295#c5, it was the first thing we tried. I think we could get around this issue by injecting the <a @download> element in the actual mozbrowser displaying the content. This would enable that particular feature, but this doesn't cover the case of an app wanting to download something that is outside of it's origin.

The isPrivate flag is for future support of private downloads as part of private browsing in fxos. What it does right now is apply the private download flag to the underlying jsdownloads api and reflects this in the dom download object by have isPrivate be true. We're not sure what private browsing is going to look like, but this flag should be enough. I'm happy to add comments to note this fact.
Flags: needinfo?(aus) → needinfo?(ehsan)
Ehsan, Fabrice and I seem to be in agreement. Jonas, what do you think?

2:56:02 PM ehsan: can we just ipc into the right process and get it to start the download for us?
2:56:10 PM ehsan: (note the "just" 
2:56:28 PM auswerk: oh, man
2:56:35 PM auswerk: i hope you don't mean what i think you mean...
2:56:43 PM ehsan: I do...
2:56:52 PM ehsan: the thing is
2:56:53 PM auswerk: Ugh, gross. 
2:56:59 PM auswerk: but yes, i get why that works.
2:56:59 PM ehsan: the issue of cookies is not theoretical
2:57:10 PM ehsan: there are tons of sites which only let you download stuff after you log in
2:57:37 PM auswerk: so
2:57:42 PM ehsan: so I think we don't have a way out here
2:57:55 PM fabrice1: we could hook this up to the mozbrowser api instead
2:58:11 PM fabrice1: that would let us get the load context
2:58:15 PM auswerk: that would be so much easier
2:58:39 PM ehsan: and get the appId from the load context?
2:58:48 PM fabrice1: yeah
2:59:05 PM ehsan: but how do we lie to necko?
2:59:15 PM ehsan: let me check
2:59:17 PM auswerk: i think in this case we don't
2:59:19 PM fabrice1: no need to lie I think
2:59:23 PM ehsan: no?
2:59:31 PM auswerk: because we can get the docshell of the document of the content
2:59:36 PM ehsan: you need to stick the appId in your channel somehow right?
2:59:43 PM ehsan: auswerk: o_O
2:59:46 PM auswerk: which i can use to loadURI within the right context
2:59:58 PM fabrice1: ehsan: yes, you can set the loadcontext on the channel
3:00:03 PM ehsan: auswerk: I was assuming we don't have access to the docshell
3:00:16 PM ehsan: ok yeah that will work
3:00:24 PM ehsan: or, it's possible to make it work at least 
3:00:27 PM auswerk: if it was in the mozbrowserapi i would on the implementation side.
3:00:57 PM ehsan: actually forget what I said, the load context _is_ the docshell 
3:01:07 PM auswerk: sooo
3:01:25 PM auswerk: this feels good from a technical perspective and functionally it's awesome!
3:01:38 PM auswerk: but, is it weird to have a 'download' method on mozbrowser?
3:02:08 PM auswerk: it doesn't actually feel that odd to me
Flags: needinfo?(ehsan)
Also, as I said later on IRC, I still think we should not do the isPrivate bit for now, and wait until we decide to implement proper support for PB.
Is the proposal to stick a download(url) function in the Browser API? I.e. make it some function on the <iframe mozbrowser>?

If so that sounds good to me.

Just make sure that we always use the cookiejar that the contents of the <iframe mozbrowser> would use.

This is likely something that the system app will need in order to allow triggering downloads from inside a particular app. So make it work for <iframe mozbrowser mozapp> as well!
Flags: needinfo?(jonas)
(In reply to Jonas Sicking (:sicking) from comment #10)
> Is the proposal to stick a download(url) function in the Browser API? I.e.
> make it some function on the <iframe mozbrowser>?
> 
> If so that sounds good to me.

Yep, it sure is! I'll make it so!

> Just make sure that we always use the cookiejar that the contents of the
> <iframe mozbrowser> would use.

Yes, that's the main reason we're going to add it to the Browser API. It will give us access to the right load context to get the correct cookie jar set on the channel.

> This is likely something that the system app will need in order to allow
> triggering downloads from inside a particular app. So make it work for
> <iframe mozbrowser mozapp> as well!

Yep! Absolutely!

I'm hoping to have an updated patch early next week with this implemented.

Thanks everyone. :)
Updating Summary to reflect what will actually be done.
Summary: [Downloads API] Add a method to the Downloads API to trigger a download from a simple URL → [Browser API] Add a method to download a file
Whiteboard: [systemsfe][p=5] → [systemsfe][p=8]
Target Milestone: 2.0 S1 (9may) → 2.0 S2 (23may)
Attachment #8418457 - Attachment is obsolete: true
Comment on attachment 8423467 [details] [diff] [review]
Patch - v1 - Add 'download' method to Browser API

Note that the intention of the method is to use the cookie jar of the content window of the web page being displayed.
Attachment #8423467 - Flags: review?(kchen)
Blocks: 906262
Comment on attachment 8423467 [details] [diff] [review]
Patch - v1 - Add 'download' method to Browser API

Review of attachment 8423467 [details] [diff] [review]:
-----------------------------------------------------------------

Nit: trailing white spaces.

We need to test this.

::: dom/browser-element/BrowserElementChildPreload.js
@@ +992,5 @@
>      webNav.stop(webNav.STOP_NETWORK);
>    },
>  
> +  _recvDownload: function(data) {
> +    let url = new content.URL(data.json.url);

It will throw here if the url is not valid.

I think we should make this API return a DOMRequest that fires onsuccess when the download finishes and onerror when the URL is invalid or channel error occurs, etc.

@@ +1040,5 @@
> +    // XXX We would set private browsing information prior to calling this.
> +    channel.notificationCallbacks = docShell.QueryInterface(Ci.nsIInterfaceRequestor);
> +
> +    // Since we're downloading our own local copy we'll want to bypass the
> +    // cache and local cache if the channel let's us specify this.

I don't understand why we would want to bypass the cache. Could you elaborate it in the comment?
Attachment #8423467 - Flags: review?(kchen)
(In reply to Kan-Ru Chen [:kanru] from comment #15)
> Comment on attachment 8423467 [details] [diff] [review]
> Patch - v1 - Add 'download' method to Browser API
> 
> Review of attachment 8423467 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Nit: trailing white spaces.

Indeed, got rid of those.

> 
> We need to test this.
>

I will add a mochitest for this method.

> 
> ::: dom/browser-element/BrowserElementChildPreload.js
> @@ +992,5 @@
> >      webNav.stop(webNav.STOP_NETWORK);
> >    },
> >  
> > +  _recvDownload: function(data) {
> > +    let url = new content.URL(data.json.url);
> 
> It will throw here if the url is not valid.

There's a few things I should probably do here at the same time for the next patch.

* Catch invalid URL exceptions and reject the promise (or DOMRequest, see next talking point :)).
* Ensure that there is a filename. If not, reject.
* Ensure that there is a file extension (or forcibly add one via Mime Type Sniffing).

> 
> I think we should make this API return a DOMRequest that fires onsuccess
> when the download finishes and onerror when the URL is invalid or channel
> error occurs, etc.
> 

Would a Promise be acceptable here as well?

> @@ +1040,5 @@
> > +    // XXX We would set private browsing information prior to calling this.
> > +    channel.notificationCallbacks = docShell.QueryInterface(Ci.nsIInterfaceRequestor);
> > +
> > +    // Since we're downloading our own local copy we'll want to bypass the
> > +    // cache and local cache if the channel let's us specify this.
> 
> I don't understand why we would want to bypass the cache. Could you
> elaborate it in the comment?

It's the pattern that we follow in Firefox (Desktop, Android) when the user specifically requests downloading a file. I'm open to changing this but I opted to follow this standard in the initial implementation.
(In reply to Ghislain Aus Lacroix [:aus] from comment #16)
> > I think we should make this API return a DOMRequest that fires onsuccess
> > when the download finishes and onerror when the URL is invalid or channel
> > error occurs, etc.
> > 
> 
> Would a Promise be acceptable here as well?

I would like to use Promise as well. However, other mozbrowser APIs are still using DOMRequest so we shouldn't make one different :)
I'm totally fine with using Promise even though other functions on the same API uses DOMRequest.

Up to what gaia devs prefer really.
Keywords: dev-doc-needed
It'll be easier for the time being to keep the interface consistent, it will return a DOMRequest.
Target Milestone: 2.0 S2 (23may) → 2.0 S3 (6june)
Blocks: 929532
Blocks: 876376
This patch omits a test for the new method. I will provide a follow-up patch with a mochitest for the new method.

This method executes in the parent to avoid needless back and forth between parent and child. This also makes all StreamListener methods work as expected which is what was required to fire success/error on the DOMRequest. The context for the connection is still the frame so the appropriate cookie jar is used.
Attachment #8423467 - Attachment is obsolete: true
Attachment #8434456 - Flags: review?(kchen)
Same as previous patch but with mochitests.
Attachment #8434456 - Attachment is obsolete: true
Attachment #8434456 - Flags: review?(kchen)
Attachment #8434536 - Flags: review?(kchen)
Comment on attachment 8434536 [details] [diff] [review]
Patch - v2 - Add 'download' method to Browser API

Review of attachment 8434536 [details] [diff] [review]:
-----------------------------------------------------------------

Please find someone to review the DownloadCore.jsm change.

Nit: trailing white spaces

You moved the download handling to the parent side. Would it still use the correct cookie jar if we initiate the download in parent?

Since we want the download to use the correct cookie jar, please test it in the test.

::: dom/browser-element/mochitest/browserElement_Download.js
@@ +1,4 @@
> +/* Any copyright is dedicated to the public domain.
> +   http://creativecommons.org/publicdomain/zero/1.0/ */
> +
> +// Bug 983747 - FILL IN TEST DESCRIPTION

Fill in test description :)

@@ +8,5 @@
> +browserElementTestHelpers.setEnabledPref(true);
> +browserElementTestHelpers.addPermission();
> +
> +var iframe;
> +var downloadURL = 'http://test/tests/dom/browser-element/mochitest/file_bug983747.sjs';

Forgot to attach this file? Please rename it to file_download_bin.sjs and rename file_bug983747.bin to file_test.bin

@@ +16,5 @@
> +  SpecialPowers.wrap(iframe).mozbrowser = true;
> +
> +  document.body.appendChild(iframe);
> +
> +  var req = iframe.download(downloadURL, { filename: 'test.bin' });

You have to wait mozbrowserloadend here before you could call the mozbrowser API.

@@ +18,5 @@
> +  document.body.appendChild(iframe);
> +
> +  var req = iframe.download(downloadURL, { filename: 'test.bin' });
> +  req.onsuccess = function() {
> +    SimpleTest.finish();

Please add one ok(true, ...) before finish so it's obvious that test has actually been run.
Attachment #8434536 - Flags: review?(kchen)
(In reply to Ghislain Aus Lacroix [:aus] from comment #20)
> Created attachment 8434456 [details] [diff] [review]
> Patch - v2 - Add 'download' method to Browser API
> 
> This patch omits a test for the new method. I will provide a follow-up patch
> with a mochitest for the new method.
> 
> This method executes in the parent to avoid needless back and forth between
> parent and child. This also makes all StreamListener methods work as
> expected which is what was required to fire success/error on the DOMRequest.
> The context for the connection is still the frame so the appropriate cookie
> jar is used.

The context in the parent side is actually the embedder's so I think it doesn't use the appropriate cookie jar.
(In reply to Kan-Ru Chen [:kanru] from comment #24)
> (In reply to Ghislain Aus Lacroix [:aus] from comment #20)
> > Created attachment 8434456 [details] [diff] [review]
> > Patch - v2 - Add 'download' method to Browser API
> > 
> > This patch omits a test for the new method. I will provide a follow-up patch
> > with a mochitest for the new method.
> > 
> > This method executes in the parent to avoid needless back and forth between
> > parent and child. This also makes all StreamListener methods work as
> > expected which is what was required to fire success/error on the DOMRequest.
> > The context for the connection is still the frame so the appropriate cookie
> > jar is used.
> 
> The context in the parent side is actually the embedder's so I think it
> doesn't use the appropriate cookie jar.

It's a little confusing but ultimately it looked to me like the TabParent is what was used to determine context, in which case, doing it from the parent should be OK? I'll ask for help with verifying this tomorrow.

I'll also fix the tests tomorrow... It was definitely a rush job, I just wanted to get one run on try before you reviewed.
(In reply to Ghislain Aus Lacroix [:aus] from comment #25)
> (In reply to Kan-Ru Chen [:kanru] from comment #24)
> > (In reply to Ghislain Aus Lacroix [:aus] from comment #20)
> > > Created attachment 8434456 [details] [diff] [review]
> > > Patch - v2 - Add 'download' method to Browser API
> > > 
> > > This patch omits a test for the new method. I will provide a follow-up patch
> > > with a mochitest for the new method.
> > > 
> > > This method executes in the parent to avoid needless back and forth between
> > > parent and child. This also makes all StreamListener methods work as
> > > expected which is what was required to fire success/error on the DOMRequest.
> > > The context for the connection is still the frame so the appropriate cookie
> > > jar is used.
> > 
> > The context in the parent side is actually the embedder's so I think it
> > doesn't use the appropriate cookie jar.
> 
> It's a little confusing but ultimately it looked to me like the TabParent is
> what was used to determine context, in which case, doing it from the parent
> should be OK? I'll ask for help with verifying this tomorrow.

The |this._window| in BrowserElementParent is the parent window, usually the Browser app or System app in Gaia. They don't have a TabParent.

It seems you just have to use this._frameLoader as the load context. So

    let interfaceRequestor =
        this._frameLoader.loadContext.QueryInterface(Ci.nsIInterfaceRequestor);
    channel.notificationCallbacks = interfaceRequestor;

might work.
> The |this._window| in BrowserElementParent is the parent window, usually the
> Browser app or System app in Gaia. They don't have a TabParent.
> 
> It seems you just have to use this._frameLoader as the load context. So
> 
>     let interfaceRequestor =
>        
> this._frameLoader.loadContext.QueryInterface(Ci.nsIInterfaceRequestor);
>     channel.notificationCallbacks = interfaceRequestor;
> 
> might work.

Gotcha, that does seem to do the trick. I'll try and figure out how to determine in the test that that is also the case.
Review comments addressed. Confirmed proper usage of cookie jar as well. The only thing that is missing is somehow testing that the cookie jar we want is set.

Paolo, there is a small change to DownloadCore.jsm which I would like you to review as well before landing. :)
Attachment #8434536 - Attachment is obsolete: true
Attachment #8435070 - Flags: review?(paolo.mozmail)
Attachment #8435070 - Flags: review?(kchen)
Comment on attachment 8435070 [details] [diff] [review]
Patch - v3 - Add 'download' method to Browser API

(In reply to Ghislain Aus Lacroix [:aus] from comment #28)
> Paolo, there is a small change to DownloadCore.jsm which I would like you to
> review as well before landing. :)

Looks straightforward, thanks!

There's actually a better syntax I would recommend here for catching selectively:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/src/DownloadCore.jsm#1969
Attachment #8435070 - Flags: review?(paolo.mozmail) → review+
(In reply to :Paolo Amadini from comment #30)
> Comment on attachment 8435070 [details] [diff] [review]
> Patch - v3 - Add 'download' method to Browser API
> 
> (In reply to Ghislain Aus Lacroix [:aus] from comment #28)
> > Paolo, there is a small change to DownloadCore.jsm which I would like you to
> > review as well before landing. :)
> 
> Looks straightforward, thanks!
> 
> There's actually a better syntax I would recommend here for catching
> selectively:
> 
> http://mxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/
> src/DownloadCore.jsm#1969

Ah yes! I'll update the syntax. Thanks!
Comment on attachment 8435070 [details] [diff] [review]
Patch - v3 - Add 'download' method to Browser API

Review of attachment 8435070 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

Add file_download_bin.sjs to mochitest.ini
Attachment #8435070 - Flags: review?(kchen) → review+
Blocks: 1022281
The test itself is exposing a flaw in HTTP channel diversion. I filed a follow-up bug to fix this issue during stabilization. In the meantime, the API method itself is usable, as expected when running under normal conditions in B2G and B2G Desktop (ergo Firefox should also work as expected).
When you pushed to b2g-inbound, you accidentally pushed yet-to-be merged mozilla-central changesets there as well as the one changeset from this bug:
https://tbpl.mozilla.org/?tree=B2g-Inbound&rev=ceb784b90138

Those jobs were then cancelled after you pushed a backout of the changeset from this bug, however we still needed the jobs, since that push brought more than just 1 changeset. 

Please please please ask before mass-cancelling on non-try and also remember to use "hg outgoing" if ever in doubt as to what will be pushed (or to where) - since the results can often be quite problematic to resolve (eg worst-case merge conflicts or having to strip changesets). Thanks :-)
This also broke vcs-sync with a non-ffwd push until :edmorley fixed it.
Pushed to b2g-inbound: https://hg.mozilla.org/integration/b2g-inbound/rev/b7d2b59e0eb9

Will get marked resolved fixed when it lands in it's final destination which is Aurora.
Target Milestone: 2.0 S3 (6june) → 2.0 S4 (20june)
Comment on attachment 8435070 [details] [diff] [review]
Patch - v3 - Add 'download' method to Browser API

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 876376 982295
User impact if declined: Functionality that exists is broken under many scenarios
Testing completed (on m-c, etc.): try, b2g-inbound 
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: 'download' method added to Browser API. This is already tracked with keyword 'dev-doc-needed'.
Attachment #8435070 - Flags: approval-mozilla-aurora?
Attachment #8435070 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Checkin needed on Aurora, see https://bugzilla.mozilla.org/show_bug.cgi?id=983747#c37 for changeset.
Keywords: checkin-needed
For perpetuity, this did land on m-c.
https://hg.mozilla.org/mozilla-central/rev/b7d2b59e0eb9
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Apologies... I somehow landed the wrong version of the patch on b2g-inbound. I've landed a follow-up here: https://hg.mozilla.org/integration/b2g-inbound/rev/ec628845dd82 

It will need merge to central and aurora. I didn't want to mess up the flags so I NI?'d you instead. I hope that's OK.
Flags: needinfo?(kwierso)
Keywords: checkin-needed
(In reply to Ghislain Aus Lacroix [:aus] from comment #43)
> Apologies... I somehow landed the wrong version of the patch on b2g-inbound.
> I've landed a follow-up here:
> https://hg.mozilla.org/integration/b2g-inbound/rev/ec628845dd82 
> 
> It will need merge to central and aurora. I didn't want to mess up the flags
> so I NI?'d you instead. I hope that's OK.

Can you attach that followup as a patch here and flag it for aurora approval?
Flags: needinfo?(kwierso)
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 983747
User impact if declined: Download method on Browser API is non-functional.
Testing completed (on m-c, etc.): On Flame device
Risk to taking this patch (and alternatives if risky): Entire method needs to be backed out from gecko and feature backed out from gaia.
String or IDL/UUID changes made by this patch: None.
Attachment #8443874 - Flags: approval-mozilla-aurora?
Duplicate of this bug: 1028475
Comment on attachment 8443874 [details]
Patch - Correct mis-landing.

Aurora approval granted.
Attachment #8443874 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Blocks: 1025853
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.