Closed
Bug 782342
Opened 12 years ago
Closed 12 years ago
blob: protocol no Content-Length header
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: benjamin.bernard, Assigned: baku)
References
()
Details
Attachments
(2 files, 2 obsolete files)
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:14.0) Gecko/20100101 Firefox/14.0.1 Build ID: 20120713224749 Steps to reproduce: I load an ogg blob using his blob URL into an <audio></audio> player. Actual results: I can't get the duration of the audio file, it's played like a streamed media. I think it's beause there is no "Content-Length" header in the Firefox implementation of the blob: protocol. Test it here : http://experiments.benvii.com/blob_content_length/ Moreover you will also noticed the "Content-Type" header is not the right one :) Same problem on "firefox for Android Beta" (v15). Expected results: I should have got the duration of the audio file, a "Content-Length" header and a "Content-Type: audio/ogg".
Reporter | ||
Updated•12 years ago
|
Andrea: Could you have a look at this bug?
Component: Untriaged → DOM
Product: Firefox → Core
Version: 14 Branch → Trunk
This is a dup.
Of Bug 730765 and the general case of Bug 583444, I believe.
Reporter | ||
Comment 4•12 years ago
|
||
It's not really a duplicate. Firefox will need to add a Content-Length header as it is going to be in the FileAPI spec. By the way I think it will also solve Bug 583444 and solve the same problem with video. You can see the bug for video here : http://experiments.benvii.com/blob_content_length/video.html See this : https://www.w3.org/Bugs/Public/show_bug.cgi?id=18548#c0 And this discussion : http://lists.w3.org/Archives/Public/public-webapps/2012JulSep/0449.html (In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #3) > Of Bug 730765 and the general case of Bug 583444, I believe.
No, we need to behave like we received a Content-Length header. We're not really serving blobs over HTTP on the local machine. I've been following the discussions.
Reporter | ||
Comment 6•12 years ago
|
||
Ok, because it's HTTP Channels. I'm not good at C++ but is it concerning this part of the source code ? http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsBlobProtocolHandler.cpp#178
We probably need to call SetContentLength there, among other things.
We also need to fake a Content-Length header in somewhere in the XMLHttpRequest code (calling getResponseHeaders on a blob URI load should have a Content-Length header).
Assignee | ||
Comment 9•12 years ago
|
||
If nobody is working on this bug, I take care of it :)
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → amarchesini
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: x86_64 → All
Assignee | ||
Comment 10•12 years ago
|
||
Thank you for the script. I converted it in a mochi test :) This patch has not been tested on try so I'm not asking for any review yet.
Attachment #651804 -
Flags: feedback?
Comment on attachment 651804 [details] [diff] [review] Bug 782342 - blob: protocol no Content-Length header Review of attachment 651804 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/base/src/nsXMLHttpRequest.cpp @@ +1465,5 @@ > + PRInt32 length; > + if (NS_SUCCEEDED(mChannel->GetContentLength(&length))) { > + aResponseHeaders.AppendLiteral("Content-Length: "); > + aResponseHeaders.AppendInt(length); > + aResponseHeaders.AppendLiteral("\r\n"); You need to make it work with GetResponseHeader as well. And you probably don't want to do this for http channels since then there's a risk that the Content-Length header shows up twice. So only do this for non-http channels. (Same in GetResonseHeader). @@ +3083,5 @@ > // ignoring return value, as this is not critical > + nsCAutoString contentType; > + if (NS_FAILED(mChannel->GetContentType(contentType)) || > + contentType.IsEmpty()) > + mChannel->SetContentType(NS_LITERAL_CSTRING("application/xml")); This doesn't look correct. GetContentType will likely always return an empty string here. What are you trying to fix here?
Assignee | ||
Comment 12•12 years ago
|
||
> You need to make it work with GetResponseHeader as well. Right. > And you probably don't want to do this for http channels since then there's > a risk that the Content-Length header shows up twice. So only do this for > non-http channels. (Same in GetResonseHeader). This code should not run for http channels. > > // ignoring return value, as this is not critical > > + nsCAutoString contentType; > > + if (NS_FAILED(mChannel->GetContentType(contentType)) || > > + contentType.IsEmpty()) > > + mChannel->SetContentType(NS_LITERAL_CSTRING("application/xml")); The previous implementation forces 'application/xml' as content type. This overwrites what we set in the blob channel. So we want to apply the default content-type ONLY if this is not already set.
> > And you probably don't want to do this for http channels since then there's > > a risk that the Content-Length header shows up twice. So only do this for > > non-http channels. (Same in GetResonseHeader). > > This code should not run for http channels. You're correct. My bad. > > > // ignoring return value, as this is not critical > > > + nsCAutoString contentType; > > > + if (NS_FAILED(mChannel->GetContentType(contentType)) || > > > + contentType.IsEmpty()) > > > + mChannel->SetContentType(NS_LITERAL_CSTRING("application/xml")); > > The previous implementation forces 'application/xml' as content type. This > overwrites what we set in the blob channel. So we want to apply the default > content-type ONLY if this is not already set. Ah. Makes sense for channels which have a contenttype even before they are opened. Please add a comment to that effect.
Assignee | ||
Comment 14•12 years ago
|
||
Let's fix the content-type issue in a separated bug. This patch is about the content-length.
Attachment #651804 -
Attachment is obsolete: true
Attachment #651804 -
Flags: feedback?
Attachment #652347 -
Flags: review?(jonas)
Comment on attachment 652347 [details] [diff] [review] Bug 782342 - blob: protocol no Content-Length header Review of attachment 652347 [details] [diff] [review]: ----------------------------------------------------------------- Sweet!
Attachment #652347 -
Flags: review?(jonas) → review+
Assignee | ||
Updated•12 years ago
|
Attachment #652347 -
Flags: checkin?
Assignee | ||
Comment 16•12 years ago
|
||
title changed.
Attachment #652347 -
Attachment is obsolete: true
Attachment #652347 -
Flags: checkin?
Attachment #654655 -
Flags: checkin?
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Updated•12 years ago
|
Attachment #654655 -
Flags: checkin?
Comment 17•12 years ago
|
||
Green on Try. https://tbpl.mozilla.org/?tree=Try&rev=3eefa652b5f9 https://hg.mozilla.org/integration/mozilla-inbound/rev/3854823c96e2
Flags: in-testsuite+
Keywords: checkin-needed
Comment 18•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3854823c96e2
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Depends on: 786399
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•