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

Make fetch "normal" #7138

Merged
merged 5 commits into from Feb 1, 2017
Merged

Make fetch "normal" #7138

merged 5 commits into from Feb 1, 2017

Conversation

jridgewell
Copy link
Contributor

Makes fetch a normal implementation. We implement #arrayBuffer by way
of utf8EncodeSync, like it should have been.

Part 1 to fixing #7119.

jridgewell added a commit to jridgewell/amphtml that referenced this pull request Jan 20, 2017
Copy link
Contributor

@mkhatib mkhatib left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@dvoytenko dvoytenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot! This makes a lot more sense.

@@ -529,7 +529,7 @@ describe('XHR', function() {
});

describe('#fetch ' + test.desc, () => {
const creative = '<html><body>This is a creative</body></html>';
const creative = '<html><body>This is a creative💩</body></html>';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change emoji.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awww. Changed.

jridgewell added a commit that referenced this pull request Feb 1, 2017
Makes fetch a normal implementation. We implement `#arrayBuffer` by way
of `utf8EncodeSync`, like it should have been.
@jridgewell jridgewell merged commit d9a1c7c into ampproject:master Feb 1, 2017
torch2424 pushed a commit to torch2424/amphtml that referenced this pull request Feb 14, 2017
torch2424 pushed a commit to torch2424/amphtml that referenced this pull request Feb 14, 2017
* Revert "Use proper application/json response header instead of arraybuffer. (ampproject#7133)"

This reverts commit 27d6886.

* Make fetch "normal"

Makes fetch a normal implementation. We implement `#arrayBuffer` by way
of `utf8EncodeSync`, like it should have been.

* Fix types

* Test UTF8 in XHR's ArrayBuffer

* Remove final uses of fetchJsonResponse
mrjoro pushed a commit to mrjoro/amphtml that referenced this pull request Apr 28, 2017
mrjoro pushed a commit to mrjoro/amphtml that referenced this pull request Apr 28, 2017
* Revert "Use proper application/json response header instead of arraybuffer. (ampproject#7133)"

This reverts commit 27d6886.

* Make fetch "normal"

Makes fetch a normal implementation. We implement `#arrayBuffer` by way
of `utf8EncodeSync`, like it should have been.

* Fix types

* Test UTF8 in XHR's ArrayBuffer

* Remove final uses of fetchJsonResponse
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants