Closed
Bug 82526
Opened 23 years ago
Closed 22 years ago
HTTP Accept: header should change based on request type
Categories
(Core :: Networking: HTTP, defect, P4)
Core
Networking: HTTP
Tracking
()
VERIFIED
DUPLICATE
of bug 170789
mozilla1.1alpha
People
(Reporter: gerv, Assigned: gerv)
References
Details
Attachments
(2 files)
2.43 KB,
patch
|
Details | Diff | Splinter Review | |
2.50 KB,
patch
|
Details | Diff | Splinter Review |
This is spun off from bug 58040 ("Accept: header should do something useful") which is now fixed. We should only put those MIME types in the Accept: header that we want - so requests from <IMG> tags should list only image types, and so on. This helps to keep the length down in the common case, and may permit the "standard" Accept: header to be a little bit longer without problems. The current pref is "network.http.accept.default", leaving room for e.g. "network.http.accept.image" and so on. The implementation of this probably requires passing the request type down into the HTTP library, or giving some other way of accessing it, and so is non-trivial. We could abuse the Prefs system to change the pref lots of times, but that may well produce problems as the pref will be read after it is set, and it may have been set to something else by that time. Gerv
Assignee | ||
Comment 1•23 years ago
|
||
After talking about this at the picnic, we decided that it would require changing an API which is pretty frozen. We can get the simple big win by doing this for images, using the imglib to set the Accept header before it calls ASyncOpen. Darin - that brain dump you gave me at the picnic - could I have it again in this bug, please? Neeti - you don't mind if I take this, do you? A suggested Accept: header for images is: image/png, image/jpeg, image/gif;q=0.2, */*;q=0.1 MNG would be added when we support it. This would be the pref "network.http.accept.image". Gerv
Comment 2•23 years ago
|
||
Gerv: Neeti is out on sabatical, so I doubt she'll mind if you take a stab at fixing this bug. A simple fix would be to add code in imgLoader.cpp just before the call to AsyncOpen. All you need to do is QI the channel to nsIHttpChannel and then call SetRequestHeader("Accept", "<accept-list>").
Assignee: neeti → gerv
Assignee | ||
Comment 3•23 years ago
|
||
I have a patch for this, but can't work out how to test it without a packet sniffer. Anyone got any ideas how I can look at the Accept: header Mozilla is sending out for requests in <IMG> tags? We get the other Accept: header if you request an image URL from the URL bar, but I would vaguely expect that... Gerv
Assignee | ||
Comment 4•23 years ago
|
||
Assignee | ||
Comment 5•23 years ago
|
||
A printf() says I'm calling the right function with the right string, so if it doesn't work, there's another bug :-) The only other issue is whether we want to do pref listener stuff here, like we do in HTTP for Accept:, so if someone XPInstalls support for a new image type, they can change the pref and we pick it up. darin? Otherwise, looking for review. Gerv
Comment 6•23 years ago
|
||
yeah... you definitely don't want to be checking the pref for each and every image load. either check the pref only once at startup or add a pref observer.
Assignee | ||
Comment 7•23 years ago
|
||
Assignee | ||
Comment 8•23 years ago
|
||
New patch attached; I've mailed pav to request review. Other reviews also welcome :-) Gerv
Comment 9•23 years ago
|
||
Since imagelib gets inited well before a user has a profile, the pref will always be the one that was put in all.js, unless I'm missing something here.
Assignee | ||
Comment 10•23 years ago
|
||
Hmm... so that basically makes it a build time, not a runtime option. What do you suggest? Do we have to do a callback thing? I'm not sure I know how to code that... Gerv
Assignee | ||
Comment 11•23 years ago
|
||
darin: can you point me at an example of setting a pref listener somewhere that I can copy? Gerv
Comment 12•23 years ago
|
||
some examples in: network/cache/src/nsCacheService.cpp network/protocol/http/src/nsHttpHandler.cpp (but, see bug 102332)
Assignee | ||
Comment 13•23 years ago
|
||
Not vital for 1.0; pushing out to aid triage. I may get to it before then, I don't know. Gerv
Target Milestone: mozilla1.0 → mozilla1.1
Comment 14•23 years ago
|
||
*** Bug 118696 has been marked as a duplicate of this bug. ***
Comment 15•23 years ago
|
||
network.http.accept.default should give lower q-values to the image types, so that a hypertext type will be retrived if both text/html and image/png are available (for instance); Mozilla is a hypertext browser first and foremost. Ideally, there would be a mechanism for adding media types to n.h.a.default, n.h.a.image, or both when they're added in Edit->Preferences->Navigator->Helper_Applications
Assignee | ||
Comment 16•23 years ago
|
||
Suggestions which do not increase the overall length of the string are welcome. Gerv
Comment 17•23 years ago
|
||
adding q-values for the image/* types is trivial to length. Allowing users to configure additional values can lead to very long Accept headers, but it's in their hands, and may be necessary for some applications.
Assignee | ||
Comment 18•23 years ago
|
||
Maybe I didn't make myself clear enough :-). The point of this bug is to _reduce_ the length of the Accept: header in some situations. It is already as long as it really should be, and will only get any longer for really important new types (such as MNG). I do not believe it is worth increasing the length to make such fine distinctions. Gerv
Comment 19•23 years ago
|
||
I'm a bit confused; mozilla is happy to send application/xhtml+xml, despite that fact that it isn't even registered yet (albeit, it is in the queue), yet adding six characters ( ;q=0.n ) is considered wasteful? I'm not proposing that Accept be populated with every media type that moz supports; however, what is in the header should clearly and correctly state the preferences of the user-agent. Otherwise, the header is useless, and might as well not be sent at all (which would save *lots* of bytes! ;) Cheers
Assignee | ||
Comment 20•23 years ago
|
||
A balance has to be struck between the utility and the length of the Accept: header. Increasing the length should only be done with good reason. Adding five bytes to every HTTP GET in case anyone wants to do content negotiation between text/html and image/png doesn't seem like a good tradeoff to me, as the situation is extraordinarily unlikely. The server operator can configure the server to prefer text/html to image/png if the q= values are equal (which he would do in this situation in any case.) Gerv
Comment 21•23 years ago
|
||
Some of the comments here do seem to miss the point... if properly implemented, this would allow the accept headers for image requests to get *shorter* rather than *longer*, as there's no need to indicate acceptance of HTML or other non-image formats in an image request. While fixing this bug (or adding this feature, whichever terminology you want to use) won't have much or any noticeable effect to users, it could produce a slight bit more impetus towards encouraging the use of content negotiation in general, which is a "chicken-or-egg" problem in that few server administrators have much interest in doing the setup at their end to serve documents in alternative formats depending on the request headers if the browsers don't do anything useful with it, while the browser developers show little interest in making more sophisticated use of the Accept header if few sites' servers do anything interesting with it. Thus, every little move at either end of the HTTP transaction helps bring this potentially useful feature more into use.
Comment 23•22 years ago
|
||
I have set up a test case where this bug prevents a page from being rendered properly, so IMHO it is actually a bug, not a feature request: http://bugzilla.ohrbelag.de/mozilla-bug-04.html
Assignee | ||
Comment 24•22 years ago
|
||
That's horribly contrived. I don't agree that it makes this issue into a bug. Gerv
Comment 25•22 years ago
|
||
Of course it is contrived. That's the point in a testcase, to make up a simple scenario where a problem can easily be seen. It is not as far-fetched as you might think, though. I ran into that bug by accident on my website, where Mozilla insisted in retrieving a style sheet instead of an image when it was supposed to negotiate between "file.png" and "file.gif". There happened to be a "file.css" in the same directory, text/css and image/png both have a q of 1.0 in the "Accept:" header, and when in doubt Apache delivers the file with the smallest size. I finally had to put style sheets and images into different directories to resolve this conflict. I could have replicated the situation described above as a test case, but chances are that this would have ended up in a "just rename file.css to a different base name, stupid" and a WONTFIX. Also note that I even set up a type-map file for the example such that the web server always delivers the image _unless_ the user agent explicitly asks for something else. This way even the most stupid of browsers that don't send "Accept:" at all would get it right. Obviously, Mozilla's "Accept:" is supposed to mean "give me application/xml in preference over text/html, or text/html in preference over text/plain, or image/png in preference over image/gif", but it also says "give me almost anything else in preference over image/gif", even when it is retrieving the resource for an IMG element, and that is plain wrong.
Comment 26•22 years ago
|
||
*** This bug has been marked as a duplicate of 170789 ***
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•