Closed Bug 170789 Opened 21 years ago Closed 21 years ago
HTTP Accept: header should be tailored to context of the request
Severity: normal → enhancement
Status: NEW → ASSIGNED
Priority: -- → P5
Target Milestone: --- → mozilla1.2beta
As far as that goes, we coudl even not list */* for CSS in standards mode (since we won't take it anyway). I agree that this would be a good idea (and would reduce the size of the Accept header). Is this just a matter of having the relevant modules call SetRequestHeader() on the channel before calling Open/AsyncOpen?
yup, that's all it takes... i'm working on a patch.
this is a duplicate
...of bug 82526 both have a patch.
*** Bug 125682 has been marked as a duplicate of this bug. ***
*** Bug 82526 has been marked as a duplicate of this bug. ***
note: this patch doesn't do any pref checking. i think that's ok for now. we can always go back over this code and add prefs if that makes sense. i figure since our code is already pretty hard coded to respond to certain MIME types, hardcoded Accept headers aren't really that bad.
Doing SetRequestHeader twice like that is odd-looking and moreover the HttpChannel idl does not make it at all clear that you have to do it like that... We should fix this API (Darin suggested just having a third aReplace param to SetRequestHeader). Not necessarily as part of this patch, but... nsStreamLoader::Init can be called from script (chrome script) and so it should really do NS_ENSURE_ARG_POINTER on at least "channel" and probably "observer" due to our "Chrome JS should not cause crashes" mantra. The rest looks quite nice.
fixed up the comments in nsIHttpChannel.idl, added some NS_ENSURE_ARG_POINTERs, and did similar Accept foo for XSLT.
Attachment #100643 - Attachment is obsolete: true
the v2 patch lacked ",*/*;q=0.1" for XSLT loads.
Attachment #100653 - Attachment is obsolete: true
Comment on attachment 100654 [details] [diff] [review] v2.1 patch -- minor tweak sr=bzbarsky
Attachment #100654 - Flags: superreview+
Should XBM be added to the image list, since Mozilla can handle them just fine? image/x-xbitmap is the mime type.
I would say "no". For the same reason that bmp and ico should not be. If people want to send that crap over the web, fine. But we should not ask them to.
i agree, the Accept header says what we prefer to handle... it doesn't have to enumerate all the data types we do handle.
As Guardian of the Accept Header (;-)), I'm definitely in favour in principle. The problem here is that the SVG people, for example, can now no longer add their MIME type to the image Accept header. Does this patch completely abandon the pref for Accept:, or does it just use it only for "top-level" page requests? When I wrote a patch for this over in bug 82526, darin suggested I use preflisteners so these values could still be prefs. What's changed? :-) Gerv
gerv: the pref is now used only for toplevel page loads. hmm... i did not realize that the SVG folks were overriding the pref settings. that worries me a bit, since the present of SVG increases the size of HTTP requests. of course, that's the whole point of the Accept header :-/ ok, so i should probably incorporate your patch for the image case. i don't think there's any reason right now to add prefs for the others like CSS, JS, and XSLT. agreed?
> the pref is now used only for toplevel page loads. hmm... i did not realize > that the SVG folks were overriding the pref settings. They may not be right now, but if they aren't then they'd like to. This is the obvious way of telling which Mozilla builds support SVG. > that worries me a bit, > since the present of SVG increases the size of HTTP requests. of course, > that's the whole point of the Accept header :-/ The increase will be more than offset by the decrease caused by this patch. We've lived with a large Accept: header for everything for ages, so it can't be that big a deal. Brendan told me (possibly partly in jest) to keep an eye on its size, and I have been doing. > ok, so i should probably incorporate your patch for the image case. i don't Don't flatter me - my patch was pretty hacky :-) > think there's any reason right now to add prefs for the others like CSS, JS, > and XSLT. agreed? Agreed. Unlike "images", CSS, JS and XSLT are individual file types by themselves. The CSS Accept: header is CSS only, right - it's not used for all "stylesheets"? Gerv
right, i believe that's correct. the CSS loader only loads CSS :)
i've been thinking some more about how to get this right for imagelib, and while a preference would work, it almost seems like it would be better to utilize the fact that imagelib decoders must be registered. we could add an attribute to imgIDecoder stating its MIME-type and whether or not it should be advertized in the Accept header. we'd just need a way to iterate the set of registered decoders.
Comment on attachment 100654 [details] [diff] [review] v2.1 patch -- minor tweak r=dougt
Attachment #100654 - Flags: review+
dougt and i spoke a bit about either using the category manager straight or possibly defining some service that could manage the Accept header. solving this definitely seems like another bug IMO, so i'm going to go ahead now with the patch as is.
fixed-on-trunk but 125682 seems to be exactly the bug for dealing with the SVG problem (in fact, that's why it was filed in the first place).
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
> we could add an attribute to >imgIDecoder stating its MIME-type and whether or not it should be advertized in >the Accept header. bug 104906 seems related.
-> tever, needs verification
QA Contact: httpqa → tever
You need to log in before you can comment on or make changes to this bug.