Closed Bug 1417463 Opened 2 years ago Closed 11 months ago

consider changing default accept header to */*

Categories

(Core :: Networking: HTTP, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox65 - wontfix
firefox66 --- fixed

People

(Reporter: bkelly, Assigned: baku)

References

(Regressed 1 open bug)

Details

(Keywords: dev-doc-complete, site-compat, Whiteboard: [necko-triaged])

Attachments

(3 files, 1 obsolete file)

Currently our default accept header is:

text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
https://searchfox.org/mozilla-central/source/modules/libpref/init/all.js#1708

This is an odd default because we should really only be sending something like this for document requests. See step 1.3 here:

https://fetch.spec.whatwg.org/#fetching

Can we change our default accept header to */* and make nsDocShell set the text/html based value as an override?
Hi Patrick, can you handle this?
Flags: needinfo?(mcmanus)
ben - I'm fine with this change. would you make it?
Flags: needinfo?(mcmanus)
Yea, I can make the change now that I have necko team blessing.  I'll try to do it in the FF59 time frame.
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Priority: -- → P3
Whiteboard: [necko-triaged]
Attached patch default_accept.patch (obsolete) — Splinter Review
I'm going to write a test in a separate patch.
Assignee: ben → amarchesini
Attachment #9034456 - Flags: review?(honzab.moz)
Duplicate of this bug: 1416392
Attached patch part 1 - neckoSplinter Review
Attachment #9034456 - Attachment is obsolete: true
Attachment #9034456 - Flags: review?(honzab.moz)
Attachment #9034569 - Flags: review?(honzab.moz)
By spec, imglib uses the "wrong" content-type but I want to talk with image peers before changing it. Maybe it would be a follow up.
Attachment #9034570 - Flags: review?(honzab.moz)
Attached patch part 3 - testsSplinter Review
Attachment #9034571 - Flags: review?(honzab.moz)
Comment on attachment 9034569 [details] [diff] [review]
part 1 - necko

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

thanks for doing this!
Attachment #9034569 - Flags: review?(honzab.moz) → review+
Attachment #9034570 - Flags: review?(honzab.moz) → review+
Comment on attachment 9034571 [details] [diff] [review]
part 3 - tests

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

::: netwerk/test/mochitests/test_accept_header.html
@@ +5,5 @@
> +  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
> +  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
> +</head>
> +<body>
> +<script>

please write a comment what this test does (reasonably deep list of steps it takes) - like what the ?get does (how you are getting the accept request header back from the server.)  it took a while to decipher this.
Attachment #9034571 - Flags: review?(honzab.moz) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/74e1cead8dcf
Default accept header should follow the fetch spec, r=mayhemer
https://hg.mozilla.org/integration/mozilla-inbound/rev/fdf7342d19f7
Default accept header should follow the fetch spec, r=mayhemer
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a7dce7a42ea
Default accept header should follow the fetch spec - tests, r=mayhemer
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

[Tracking Requested - why for this release]:

It would be nice to uplift this; I am seeing comments on Twiter that saving images results in un-openable images (which I verified was fixed by this bug).

https://twitter.com/LifeTimeCooking/status/1094488840067178496

Saving the Indian French Toast with Baked Strawberries image results in a corrupt image.

Oops, missed the URL in the last comment: https://vegeyum.wordpress.com/

Sorry, this is an old issue and the patches involved are more risky than we can accept for a dot release. This fix will need to ride with Fx66.

Depends on: 1526731
Blocks: 1530887
Duplicate of this bug: 1532223
Duplicate of this bug: 1533990
No longer depends on: 1526731
Regressions: 1526731

Plus :

network.http.accept.default is not handled nor editable anymore

https://developer.mozilla.org/en-US/docs/Web/HTTP/Content_negotiation/List_of_default_Accept_values

(In reply to webmaster from comment #19)

Hello,

Should you have a look here : https://github.com/apache/incubator-pagespeed-mod/issues/1857#issuecomment-480581634 .

Thanks,

Eric

(In reply to moreeasy from comment #21)

"image/webp" accept header is gone in ff66, despite https://developer.mozilla.org/en-US/docs/Web/HTTP/Content_negotiation/List_of_default_Accept_values

Removing image/webp was probably unintentional and should be the subject of a new bug (if it hasn't been filed already).

Regressions: 1544231

:cmills Please see comment 20 and comment 21

(In reply to jscher2000 from comment #22)

Removing image/webp was probably unintentional and should be the subject of a new bug (if it hasn't been filed already).

Bug 1544231

Flags: needinfo?(cmills)
No longer regressions: 1544231

(In reply to Gingerbread Man from comment #23)

:cmills Please see comment 20 and comment 21

(In reply to jscher2000 from comment #22)

Removing image/webp was probably unintentional and should be the subject of a new bug (if it hasn't been filed already).

Bug 1544231

Thanks for bringing this to my attention. I just want to verify what I need to change here. Is this just a straight revert of what I did previously, or is there more to it?

Flags: needinfo?(cmills) → needinfo?(gingerbread_man)

The needinfo request was for updating the List_of_default_Accept_values article. You never touched that one, so it wasn't a case of reverting your changes. It has since been updated by jscher2000 and appears to be in order now. If you could double-check, I'd appreciate it.

Flags: needinfo?(gingerbread_man)

(In reply to Gingerbread Man from comment #25)

The needinfo request was for updating the List_of_default_Accept_values article. You never touched that one, so it wasn't a case of reverting your changes. It has since been updated by jscher2000 and appears to be in order now. If you could double-check, I'd appreciate it.

Ah, ok, thanks! And sorry for the confusion.

I've checked it, and it looks fine.

Is it no longer possible to configure the Accept header?

I work a lot with web APIs and being able to prefer JSON over XML was a huge plus for Firefox compared to say Chrome. Especially with Firefoxs built in JSON viewer.

Hello,

(In reply to jscher2000 from comment #22)

Removing image/webp was probably unintentional and should be the subject of a new bug (if it hasn't been filed already).

https://bugzilla.mozilla.org/show_bug.cgi?id=1544231

Thanks,

Eric

Regressions: 1560896
You need to log in before you can comment on or make changes to this bug.