Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change Response's statusText's default #836

Merged
merged 1 commit into from Nov 21, 2018

Conversation

3 participants
@annevk
Copy link
Member

annevk commented Nov 20, 2018

The empty string is a better default as the field is optional in HTTP and rather meaningless. This way if you change the status and forget about statusText it won't give the mistaken impression things are OK.

Tests: ...

Fixes #698.


Preview | Diff

Change Response's statusText's default
The empty string is a better default as the field is optional in HTTP and rather meaningless. This way if you change the status and forget about statusText it won't give the mistaken impression things are OK.

Tests: ...

Fixes #698.

annevk added a commit to web-platform-tests/wpt that referenced this pull request Nov 20, 2018

Fetch: change Response's statusText's default
See whatwg/fetch#836 for context.

This also cleans up the code a bit.
@annevk

This comment has been minimized.

Copy link
Member Author

annevk commented Nov 20, 2018

@youennf @yutakahirano what do you think? With browser bugs this is reasonable to do? It's an API change, but extremely minor.

@domfarolino

This comment has been minimized.

Copy link
Member

domfarolino commented Nov 20, 2018

FWIW I also think this makes sense, but do wonder how much potentially older JS out there is relying on "OK".

Ms2ger added a commit to web-platform-tests/wpt that referenced this pull request Nov 20, 2018

Fetch: change Response's statusText's default
See whatwg/fetch#836 for context.

This also cleans up the code a bit.
@annevk

This comment has been minimized.

Copy link
Member Author

annevk commented Nov 20, 2018

It seems unlikely given this only affects synthetic responses and you'd generally not check this field (or set it), but yes.

@domfarolino

This comment has been minimized.

Copy link
Member

domfarolino commented Nov 20, 2018

Oh that's right, good point.

@annevk annevk merged commit a3d423f into master Nov 21, 2018

2 checks passed

Participation annevk participates on behalf of Mozilla Corporation
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@annevk annevk deleted the annevk/change-default-Response-statusText branch Nov 21, 2018

@jdm

This comment has been minimized.

Copy link
Collaborator

jdm commented Nov 21, 2018

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 26, 2018

Bug 1508658 [wpt PR 14144] - Fetch: change Response's statusText's de…
…fault, a=testonly

Automatic update from web-platform-testsFetch: change Response's statusText's default

See whatwg/fetch#836 for context.

This also cleans up the code a bit.

--

wpt-commits: 99e9e996267ccd5c1344b7d0164c5e5763b3b174
wpt-pr: 14144

jankeromnes pushed a commit to jankeromnes/gecko that referenced this pull request Nov 27, 2018

Bug 1508658 [wpt PR 14144] - Fetch: change Response's statusText's de…
…fault, a=testonly

Automatic update from web-platform-testsFetch: change Response's statusText's default

See whatwg/fetch#836 for context.

This also cleans up the code a bit.

--

wpt-commits: 99e9e996267ccd5c1344b7d0164c5e5763b3b174
wpt-pr: 14144

aarongable pushed a commit to chromium/chromium that referenced this pull request Nov 30, 2018

Change ResponseInit default statusText
From "OK" => "", as per the spec change
whatwg/fetch#836.

R=yhirano@chromium.org, yoav@yoav.ws

Bug: 907441
Change-Id: Id58718ecdba49432078358c20d62df5ff0eda8f8
Reviewed-on: https://chromium-review.googlesource.com/c/1348739
Commit-Queue: Dominic Farolino <domfarolino@gmail.com>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612662}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.