Note: There are a few cases of duplicates in user autocompletion which are being worked on.

HTTP Accept: header should be tailored to context of the request

RESOLVED FIXED in mozilla1.2beta

Status

()

Core
Networking: HTTP
P5
enhancement
RESOLVED FIXED
15 years ago
5 years ago

People

(Reporter: Darin Fisher, Assigned: Darin Fisher)

Tracking

({embed})

Trunk
mozilla1.2beta
embed
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [SNAP])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

15 years ago
HTTP Accept: header should be tailored to context of the request.  This impacts
imagelib, CSS loader, and JS loader, (potentially) like so:

For inline image loads:
  Accept: video/x-mng,image/png,image/jpeg,image/gif;q=0.2,*/*;q=0.1

For CSS loads:
  Accept: text/css,*/*;q=0.1

For JS loads:
  Accept: */*

This would really help reduce the amount of data we send w/ each request.  (I
think Accept: */* makes sense for JS loads since our current Accept: header
value doesn't mention any preference for javascript content.)

I know there is a bug already about doing this for imagelib, but I think we
should really do this for CSS and JS as well.  There may also be some benefit to
doing this for plugins too.
(Assignee)

Updated

15 years ago
Severity: normal → enhancement
Status: NEW → ASSIGNED
Keywords: embed
Priority: -- → P5
Whiteboard: [SNAP]
Target Milestone: --- → mozilla1.2beta

Comment 1

15 years ago
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?
(Assignee)

Comment 2

15 years ago
yup, that's all it takes... i'm working on a patch.
(Assignee)

Comment 3

15 years ago
Created attachment 100643 [details] [diff] [review]
v1 patch

please review
this is a duplicate
...of bug 82526

both have a patch.
(Assignee)

Comment 6

15 years ago
*** Bug 125682 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 7

15 years ago
*** Bug 82526 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 8

15 years ago
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.

Comment 9

15 years ago
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.
(Assignee)

Comment 10

15 years ago
Created attachment 100653 [details] [diff] [review]
v2 patch

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
(Assignee)

Comment 11

15 years ago
Created attachment 100654 [details] [diff] [review]
v2.1 patch -- minor tweak

the v2 patch lacked ",*/*;q=0.1" for XSLT loads.
Attachment #100653 - Attachment is obsolete: true

Comment 12

15 years ago
Comment on attachment 100654 [details] [diff] [review]
v2.1 patch -- minor tweak

sr=bzbarsky
Attachment #100654 - Flags: superreview+

Comment 13

15 years ago
Should XBM be added to the image list, since Mozilla can handle them just 
fine?  image/x-xbitmap is the mime type.

Comment 14

15 years ago
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.
(Assignee)

Comment 15

15 years ago
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
(Assignee)

Comment 17

15 years ago
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
(Assignee)

Comment 19

15 years ago
right, i believe that's correct. the CSS loader only loads CSS :)
(Assignee)

Comment 20

15 years ago
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 21

15 years ago
Comment on attachment 100654 [details] [diff] [review]
v2.1 patch -- minor tweak

r=dougt
Attachment #100654 - Flags: review+
(Assignee)

Comment 22

15 years ago
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.
(Assignee)

Comment 23

15 years ago
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
Last Resolved: 15 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.

Comment 25

15 years ago
-> tever, needs verification
QA Contact: httpqa → tever
You need to log in before you can comment on or make changes to this bug.