Closed Bug 945323 Opened 8 years ago Closed 7 years ago
[Download API] Downloaded path isn't honoring download attribute
Apply the patch in bug 945099 & run that mochitest. You'll notice that the download path uses serve_file.sjs rather than test.bin. This means that the download API isn't honoring the download attribute specified in the anchor tag for determining the file name of the downloaded file.
This also happens on device as well.
Summary: [Download API] Downloaded path isn't honoring download attribute in the emulator → [Download API] Downloaded path isn't honoring download attribute
This will block shipping the download manager given that this is a core piece of the web not working here.
blocking-b2g: --- → 1.4?
Assignee: nobody → aus
Status: NEW → ASSIGNED
It actually works as expected for me and the test will ensure that it stays that way.
Attachment #8360584 - Flags: review?(kyle) → review+
Added reviewer to commit message. r+ carries over. r=qdot
Attachment #8360698 - Flags: review+
Attachment #8360584 - Attachment is obsolete: true
Yep, totally still broken in the emulator. That sucks. Investigating...
Somehow, our HelperAppDialog.js that specifically causes all files to be saved to disk must be getting the wrong default filename, I'm trying to confirm this now. I am, however, having trouble finding where "promptForSaveToFile" actually gets called.
I've traced through the entire code path and while running on desktop, despite enabling remote tabs, it is impossible to reproduce the issue. There are no obvious areas where the code paths diverge when compiling Gonk so I'm a little bit stumped. I'm going to make a debug build of the emulator and see what's going on.
blocking-b2g: 1.4+ → 1.4?
Target Milestone: 1.3 C3/1.4 S3(31jan) → ---
Going to assume the flags were reset by accident.
blocking-b2g: 1.4? → 1.4+
Target Milestone: --- → 1.3 C3/1.4 S3(31jan)
This is an attempt at fixing the issue.
Attachment #8360698 - Attachment is obsolete: true
Target Milestone: 1.3 C3/1.4 S3(31jan) → 1.4 S1 (14feb)
Last update to the patch, I swear. :)
Comment on attachment 8370815 [details] [diff] [review] Patch - Create fake content disposition header for the child r=jst, but bz should sign off on this one too.
Comment on attachment 8370815 [details] [diff] [review] Patch - Create fake content disposition header for the child Why is this code using GetContentDispositionHeader at all? That's not reliable in all sorts of cases; there's a reason it's deprecated! Why can't we just send the contentDispositionFilename and contentDisposition to the parent instead of sending the header string?
Attachment #8370815 - Flags: review?(bzbarsky) → review-
To answer my own question, it's using that because it was the expedient thing back in bug 589292, apparently. :(
I guess we should probably send all three, actually, since we still need the header too, in theory. I wonder if we can just kill off the remaining uses of GetContentDispositionHeader.
Indeed. It's unfortunate that it's implemented the way it is right now. In theory, this issue should go away as well once we short-circuit things. Currently we need to do this because we have to go Parent->Child->Parent with the info and the *Child* is the one that didn't have the correct data. I'm OK with updating the IPDL to receive *all* 3 instead but it will add another 2 to 3 days of work (round-trip with all the reviews and the push to try before actually landing on inbound). I'll have an updated patch for this soon but I need to sort out a different 1.3 blocker bug first.
Comment on attachment 8375817 [details] [diff] [review] Patch - Update ExternalHelperAppParent and Child to also use ContentDispositionHint and ContentDispositionFilename Should the Anjuta bits really be landing as part of this bug? r=me on the rest, though I wish some of that stuff had better (any?) API documentation...
Attachment #8375817 - Flags: review?(bzbarsky) → review+
Meh, I can land the Anjuta bits separately in a bug for that purpose. Some of that stuff does have API documentation but only for the actual public interfaces that our exposed. None of the IPC specific interfaces have any documentation. Also, I get to try out my shiny new commit access. Hooray!
r=bz, final patch removes anjuta .hgignore bits.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.