HTTP Accept: header should change based on request type

VERIFIED DUPLICATE of bug 170789

Status

()

Core
Networking: HTTP
P4
normal
VERIFIED DUPLICATE of bug 170789
16 years ago
6 years ago

People

(Reporter: gerv, Assigned: gerv)

Tracking

Trunk
mozilla1.1alpha
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

16 years ago
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

Updated

16 years ago
Priority: -- → P4
Target Milestone: --- → mozilla1.0
(Assignee)

Comment 1

16 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

16 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

16 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

16 years ago
Created attachment 46164 [details] [diff] [review]
Patch v.1
(Assignee)

Comment 5

16 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

16 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

16 years ago
Created attachment 46508 [details] [diff] [review]
Patch v.2 - read pref once at startup
(Assignee)

Comment 8

16 years ago
New patch attached; I've mailed pav to request review. Other reviews also
welcome :-)

Gerv

Comment 9

16 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

16 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

16 years ago
darin: can you point me at an example of setting a pref listener somewhere that
I can copy?

Gerv

Comment 12

16 years ago
some examples in:

network/cache/src/nsCacheService.cpp
network/protocol/http/src/nsHttpHandler.cpp (but, see bug 102332)
(Assignee)

Comment 13

16 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
*** Bug 118696 has been marked as a duplicate of this bug. ***

Comment 15

16 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

16 years ago
Suggestions which do not increase the overall length of the string are welcome.

Gerv

Comment 17

16 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

16 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

16 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

16 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

16 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 22

15 years ago
qa -> tever
QA Contact: benc → tever

Comment 23

15 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

15 years ago
That's horribly contrived. I don't agree that it makes this issue into a bug. 

Gerv

Comment 25

15 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

15 years ago

*** This bug has been marked as a duplicate of 170789 ***
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → DUPLICATE

Comment 27

15 years ago
verified dupe.
Status: RESOLVED → VERIFIED
QA Contact: tever → junruh
You need to log in before you can comment on or make changes to this bug.