Closed
Bug 394416
Opened 17 years ago
Closed 16 years ago
Feed sniffing should apply to fewer mime types
Categories
(Firefox Graveyard :: RSS Discovery and Preview, defect, P5)
Firefox Graveyard
RSS Discovery and Preview
Tracking
(Not tracked)
VERIFIED
FIXED
Firefox 3.1b1
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
(Keywords: html5, perf)
Attachments
(1 file)
1.93 KB,
patch
|
sayrer
:
review+
|
Details | Diff | Splinter Review |
Right now, we sniff all loads in the content area, subframes, or <object> for possibly being feeds. This seems wrong to me. I see no reason to sniff an image/png or video/whatever or application/x-msword file to see whether it might be a feed. What we do now is bad for the web in general, not to mention a possible performance drag. Realistically, this code should only sniff types that are likely to be feeds. A few of the relevant XML types, text/plain, the "maybe text" type, and text/html should cover it, I suspect.
Flags: blocking-firefox3?
Updated•17 years ago
|
Summary: Feed sniffing too permissive → Feed sniffing should apply to fewer mime types
Updated•17 years ago
|
Assignee: nobody → rflint
OS: Linux → All
Hardware: PC → All
Target Milestone: --- → Firefox 3 M10
Updated•17 years ago
|
Target Milestone: Firefox 3 M10 → Firefox 3 M11
Updated•17 years ago
|
Priority: -- → P5
Updated•17 years ago
|
Flags: wanted-firefox3+
Flags: blocking-firefox3-
Flags: blocking-firefox3+
Target Milestone: Firefox 3 Mx → Firefox 3 M11
Assignee | ||
Comment 1•16 years ago
|
||
This also happens to violate HTML5, and is causing real-life interoperability issues...
Flags: blocking-firefox3.1?
Keywords: html5
Comment 2•16 years ago
|
||
So are you saying that you want this bug to implement HTML5 sniffing, which means only sniffing for the results of incompetent PHP programmers who send text/html while making it impossible to subscribe to static foo.rss or foo.atom if served by an Apache server with a mime.types file from before August 2007, or to subscribe to foo.xml at all?
Assignee | ||
Comment 3•16 years ago
|
||
HTML5 sniffing specifically allows you to sniff the Apache text/plain brokenness, so I have no idea what you're talking about with "impossible ... if served by an Apache server". I think what we should be doing is what I said in comment 0: text/html, XML types, and the HTML5-allowed sniffing.
Comment 4•16 years ago
|
||
Yes, HTML5 sniffing allows you to sniff Apache text/plain: as text/plain, or as application/octet-stream. Nothing else. There is only one possible entry into feed sniffing in the HTML5 algo, "text/html". No sniffing of text/xml or application/xml or any type ending +xml, including RSS 1.0 served as application/rdf+xml, is allowed.
Comment 5•16 years ago
|
||
(In reply to comment #4) > No sniffing of text/xml or > application/xml or any type ending +xml, including RSS 1.0 served as > application/rdf+xml, is allowed. If that is correct, I don't see how we can follow HTML5's current text. Do any browsers behave that way?
Comment 6•16 years ago
|
||
I don't see how HTML5 forbids "sniffing" XML for feeds. (I don't think that's considered sniffing.)
Comment 7•16 years ago
|
||
(In reply to comment #5) > Do any browsers behave that way? Of course not. (In reply to comment #6) > I don't see how HTML5 forbids "sniffing" XML for feeds. (I don't think that's > considered sniffing.) Um, what? Please, please, please *read* http://www.whatwg.org/specs/web-apps/current-work/multipage/infrastructure.html#content-type0 paying attention to parts like 2.7.2.3 where you are told that text/plain may only remain text/plain or become application/octet-stream, and 2.7.2.5 where you are told unambiguously that if you are sniffing and the type is any XML type, you must stop instantly, and then 2.7.2.7 where you are told if and only if you have made it that far without being stopped and the type is text/html, you may then proceed to the lovingly detailed CONTENT-TYPE SNIFFING: feed or HTML section, and then explain to me your theory that sniffing content served under another type and treating it as though it was served with a feed type is not sniffing. I look forward to hearing wtf that section about sniffing feeds is for, if sniffing feeds is not sniffing feeds. While you are doing that, I'll dig up Hixie's mails that clearly and unambiguously say that that's exactly what he intends to say, that nothing but application/rss+xml, application/atom+xml, and text/html may be treated as being feeds when loaded as content, rather than from autodiscovery. Whether or not at some future date HTML5 will still say that, that *is* what it says today.
Assignee | ||
Comment 8•16 years ago
|
||
That section of HTML5 used to say something much more reasonable, and it sounds like it should be changed to say that more reasonable thing again. Please post to public-html accordingly? The original point of comment 0 stands, in any case.
Comment 9•16 years ago
|
||
The file type is XML. What happens during XML page loading is defined here: http://www.whatwg.org/specs/web-apps/current-work/multipage/history.html#read-xml HTML5 does indeed not allow sniffing in text/plain. I did not contest that.
Comment 10•16 years ago
|
||
(I'm not sure what you heard from Hixie and where. It does not seem to be accurate as far as I can tell. Also, reading HTML5 is part of my job...)
Comment 11•16 years ago
|
||
Ah, my apologies, I lacked sufficient imagination to expect that there would be sniffing and "sniffing" and that "sniffing" wouldn't be called sniffing.
Comment 12•16 years ago
|
||
Would be good to get, but not going to block on it.
Flags: wanted-firefox3.1+
Flags: blocking-firefox3.1?
Flags: blocking-firefox3.1-
Assignee | ||
Comment 13•16 years ago
|
||
Attachment #335064 -
Flags: review?(rflint)
Updated•16 years ago
|
Attachment #335064 -
Flags: review?(rflint) → review+
Comment 14•16 years ago
|
||
Comment on attachment 335064 [details] [diff] [review] Let's try this >+ // Don't sniff arbitrary types. Limit sniffing to situations that >+ // we think can reasonably arise. >+ if (!contentType.EqualsLiteral(TEXT_PLAIN) && Let's take this out, per HTML5. We can put it back if it turns out not to be web compatible. >+ !contentType.EqualsLiteral(APPLICATION_GUESS_FROM_EXT) && >+ !contentType.EqualsLiteral(TEXT_HTML) && >+ !contentType.EqualsLiteral(APPLICATION_OCTET_STREAM) && >+ // Same criterion as XMLHttpRequest. Should we be checking for "+xml" >+ // and check for text/xml and application/xml by hand instead? >+ contentType.Find("xml") == -1) We can do better here, let's file a follow-up.
Assignee | ||
Comment 15•16 years ago
|
||
I'll take out the APPLICATION_GUESS_FROM_EXT too, then, since that's our "text/plain that looks like binary" thing.
Assignee | ||
Updated•16 years ago
|
Assignee: rflint → bzbarsky
Assignee | ||
Comment 16•16 years ago
|
||
Pushed changeset 45d7857e2e0e with that change.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Target Milestone: Firefox 3 beta3 → Firefox 3.1b1
Comment 17•15 years ago
|
||
Verified that it has been fixed on build Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090530 Shiretoko/3.5pre (.NET CLR 3.5.30729)
Status: RESOLVED → VERIFIED
Updated•5 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•