Closed Bug 918752 Opened 7 years ago Closed 4 years ago

[XHR2] Default Accept: header more complex than */*

Categories

(Core :: DOM: Core & HTML, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: hsteen, Assigned: wisniewskit)

References

()

Details

(Keywords: dev-doc-complete, site-compat)

Attachments

(1 file, 3 obsolete files)

If no author-supplied Accept header is given, the default should be */*
Test case:
http://w3c-test.org/web-platform-tests/master/XMLHttpRequest/send-accept.htm
Here is a simple patch that overrides accept accordingly in Send(), if it has not been user-specified.
Attachment #8760829 - Flags: review?(jst)
Note that bug 433859 is a dupe of this one. It does mention the fetch API has the same requirement, but the fetch API's related test is passing already: http://w3c-test.org/fetch/api/basic/accept-header.html

It might be worth changing that test so it also verifies that Accept can be overridden, as it only checks that */* is set by default (the same change might be worth adding to this XHR test).

Also, the Fetch spec isn't as simple as just requiring */*, but requires specific UA strings for navigation requests and certain request content types (see https://fetch.spec.whatwg.org/#concept-fetch). It seems the test isn't checking those cases... perhaps it's outdated?
(That XHR test is already checking that Accept can be overridden, no? Master repo: https://github.com/w3c/web-platform-tests/blob/master/XMLHttpRequest/send-accept.htm )
Ah, indeed it is. Strange that I would have missed that; guess I didn't expect it to test for more than one thing at once.
Comment on attachment 8760829 [details] [diff] [review]
918752-set-accept-request-header-properly-if-not-user-set-for-xhrs.diff

Review of attachment 8760829 [details] [diff] [review]:
-----------------------------------------------------------------

Changing reviewers, just in case :jst is unavailable. Hopefully this isn't a faux pas...
Attachment #8760829 - Flags: review?(jst) → review?(jonas)
Here's a successful try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ef87b0be88b2

The noted failures there are happening for me locally with or without this patch, so they should be unrelated.
Rebased.
Attachment #8760829 - Attachment is obsolete: true
Attachment #8760829 - Flags: review?(jonas)
Attachment #8766840 - Flags: review?(jst)
Rebased.
Assignee: nobody → wisniewskit
Attachment #8766840 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8766840 - Flags: review?(jst)
Attachment #8771717 - Flags: review?(jst)
Attachment #8771717 - Flags: review?(jst) → review+
Duplicate of this bug: 433859
Rebased the patch. Try seems fine: https://treeherder.mozilla.org/#/jobs?repo=try&revision=76c1042164ad

There were no functional changes, just a minor update to a test I had missed, so I'm carrying over r+ and requesting check-in.
Attachment #8771717 - Attachment is obsolete: true
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/00afd3c93ccf
Override Accept request header to */* in XMLHttpRequest::Send() if it is not specified by the user. r=jst
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/00afd3c93ccf
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Depends on: 1336786
You need to log in before you can comment on or make changes to this bug.