Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor) Reporter
	Description • 7 years ago

Follow-up per Bug 1370485 Comment 24:

"I think you're right. We might at some point hit compatibility issues of the opposite nature due to Chrome and Safari lowercasing, but waiting with lowercasing until Firefox 58's Nightly arrives seems good. And to be clear, I think we should keep the preference available throughout Firefox 58 so we can easily revert if something breaks. We can remove the preference itself in Firefox 59 or 60 or so."

	Andrew Overholt [:overholt]
	Comment 1 • 6 years ago

Shawn! /me waves o/

	Thomas Wisniewski [:twisniewski] Assignee
	Comment 2 • 6 years ago

Hi Shawn, I was wondering if you're still planning on pushing this one across the finish line? (I wouldn't mind doing so if you're busy).

	Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor) Reporter
	Comment 3 • 6 years ago

I can't make any try push, it seems my account suspended :(
Reset assignee.

	Anne (:annevk)
	Comment 4 • 6 years ago

Shawn, apparently that happens automatically after a period of inactivity. I think it should be pretty trivial for you to get it enabled again, following the guidelines at https://www.mozilla.org/en-US/about/governance/policies/commit/.

	Thomas Wisniewski [:twisniewski] Assignee
	Comment 5 • 6 years ago

I'm doing a try-push here, in the interests of helping this along: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bfe48ffef0a2fcbfffb3bae5640c1dfdefb23c3a

	Thomas Wisniewski [:twisniewski] Assignee
	Comment 6 • 6 years ago

Shawn, Anne, I don't see any unexpected failures in that try-run, so flipping the pref should be fine in that regard.

Were there any concerns we had left to address before flipping this pref that you're aware of, or should we go ahead? (I only recall us not wanting to try this out until after Firefox 57 was released).

	Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor) Reporter
	Comment 7 • 6 years ago

(In reply to Thomas Wisniewski [:twisniewski] from comment #6)
> Shawn, Anne, I don't see any unexpected failures in that try-run, so
> flipping the pref should be fine in that regard.
> 
> Were there any concerns we had left to address before flipping this pref
> that you're aware of, or should we go ahead? (I only recall us not wanting
> to try this out until after Firefox 57 was released).

If nothing goes wrong, we should remove that flag in the future, or now?

You can also get rid of meta config from web-platform-test.

https://searchfox.org/mozilla-central/source/testing/web-platform/meta/xhr/__dir__.ini

The original discussion is here:
https://github.com/whatwg/xhr/issues/146

Extra bug report from chromium:
https://bugs.chromium.org/p/chromium/issues/detail?id=749086

I don't see any further discussion from that bug. It should be fine.

	Anne (:annevk)
	Comment 8 • 6 years ago

We had some web compatibility fallout the last time around (~1 year ago), which is why we reverted the lowercasing behavior. Since then Chrome and Safari both shipped lowercasing behavior and it's seemingly compatible for them so we should try again, but keep the flag until we've done at least one stable release with lowercasing behavior to be safe.

	Thomas Wisniewski [:twisniewski] Assignee
	Comment 9 • 6 years ago

Alright, then let's just flip the pref for now and we can remove it once we've confirmed that it can go away in a release or three.

(I did another quick try-run just in case, nothing interesting stands out: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f5848d78343ccbd6f70c67376af4d3d1c2ec5d7)

	Thomas Wisniewski [:twisniewski] Assignee
	Comment 10 • 6 years ago

change default pref to on for lowercase header names in XHR.getAllResponseHeaders

	Henri Sivonen (:hsivonen)
	Comment 11 • 6 years ago

Comment on attachment 9011331 [details]
Bug 1398718 - change default pref to on for lowercase header names in XHR.getAllResponseHeaders; r?hsivonen

Henri Sivonen (:hsivonen) has approved the revision.

	Thomas Wisniewski [:twisniewski] Assignee
	Comment 12 • 6 years ago

Alright, let's do this. Requesting check-in.

	Pulsebot
	Comment 13 • 6 years ago

Pushed by ccoroiu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cc0cb1e1912d
change default pref to on for lowercase header names in XHR.getAllResponseHeaders; r=hsivonen

	Stefan Hindli [:stefan_hindli]
	Comment 14 • 6 years ago

Backed out changeset cc0cb1e1912d (Bug 1398718) for task timeout after 3600 seconds bustages CLOSED TREE

Failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed,busted,exception&classifiedState=unclassified&fromchange=787f9095543f9b60d22301d6c3ef06c4b47b1173&selectedJob=201213744

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=201213744&repo=autoland&lineNumber=74

	Andrew Overholt [:overholt]
	Comment 15 • 6 years ago

(In reply to Stefan Hindli [:stefan_hindli] from comment #14)
> Failure log:
> https://treeherder.mozilla.org/logviewer.
> html#?job_id=201213744&repo=autoland&lineNumber=74

This doesn't seem to indicate anything wrong with this particular patch but I'll let Thomas respond in case I'm missing something.

	Thomas Wisniewski [:twisniewski] Assignee
	Comment 16 • 6 years ago

Yeah, I don't see how my patch would cause that, if it passed the normal try builds.

	Pulsebot
	Comment 17 • 6 years ago

Backout by rgurzau@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/79a94ed5a62d
Backed out changeset cc0cb1e1912d for task timeout after 3600 seconds bustages CLOSED TREE

	Thomas Wisniewski [:twisniewski] Assignee
	Comment 18 • 6 years ago

Do you think it would it be ok to try pushing this again in a day or two, Andrew? Or is there a strong reason to suspect that it's really this pref-flip that's causing those failures?

	Andrew Overholt [:overholt]
	Comment 19 • 6 years ago

Yes, I think we should try pushing this again. Thanks, Thomas.

	Thomas Wisniewski [:twisniewski] Assignee
	Comment 20 • 6 years ago

Alright, I still don't see any troubling signs on a try-run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=76e29885054f0ed9f2c40ebb448b83be4826e5f6

Let's try checking in again.

	Cristian Brindusan [:cbrindusan]
	Comment 21 • 6 years ago

Unfortunately, we're unable to land this bug due to this error from Phabricator:

"This diff does not have the proper author information uploaded to Phabricator. This can happen if the diff was created using the web UI, or a non standard client. The author should re-upload the diff to Phabricator using the "arc diff" command."

Can you please take a look?

	Thomas Wisniewski [:twisniewski] Assignee
	Comment 22 • 6 years ago

Weird. I just reopened the Phabricator issue and did another arc diff.. hopefully that does the trick?

	Cristian Brindusan [:cbrindusan]
	Comment 23 • 6 years ago

Yes :)

	Pulsebot
	Comment 24 • 6 years ago

Pushed by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f49f269d4fd0
change default pref to on for lowercase header names in XHR.getAllResponseHeaders; r=hsivonen

	Stefan Hindli [:stefan_hindli]
	Comment 25 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/f49f269d4fd0

	Kohei Yoshino [:kohei]
	Comment 26 • 6 years ago

Posted site compatibility note just in case: https://www.fxsitecompat.com/en-CA/docs/2018/xhr-getallresponseheaders-now-returns-header-names-in-lowercase/

	j.j.
	Comment 27 • 6 years ago

This landed in Fx 64, right? Please set Target Milestone to avoid confusion.
Is there a bug filed to remove that flag?

	Thomas Wisniewski [:twisniewski] Assignee
	Comment 28 • 6 years ago

I've just filed bug 1504344 to track the removal of the flag. Thanks for the reminder!

	Chris Mills [:cmills]
	Comment 29 • 6 years ago

Note for docs team:

I've added a note to the Fx 64 rel notes:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/64#APIs

When documenting this, it'll just need compat data updating and a note adding to the actual ref page to explain the change.

	Chris Mills [:cmills]
	Comment 30 • 6 years ago

Documented:

I have added a note to the ref page:
https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/getAllResponseHeaders

And submitted a PR to the compat data repo to add a data point about it:
https://github.com/mdn/browser-compat-data/pull/3114