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

Update MIME type for glb files #5420

Merged
merged 5 commits into from
Jun 9, 2017
Merged

Update MIME type for glb files #5420

merged 5 commits into from
Jun 9, 2017

Conversation

emackey
Copy link
Contributor

@emackey emackey commented Jun 5, 2017

This was changed in KhronosGroup/glTF#943.

Also fixed an eslint warning in server.js.

@mramato
Copy link
Contributor

mramato commented Jun 5, 2017

Model.js has a list of accepted model mime types that also needs to be updated.

@emackey
Copy link
Contributor Author

emackey commented Jun 5, 2017

Thanks @mramato.

@mramato
Copy link
Contributor

mramato commented Jun 5, 2017

For the Model.js change, are we okay with only requesting the official mime types and not the older versions? Technically that's a breaking change (though I doubt many people rely on it).

@emackey
Copy link
Contributor Author

emackey commented Jun 5, 2017

We've got */* on the end there, so the old types aren't completely removed, they're just down at the bottom of the priority list.

@@ -154,8 +154,8 @@ define([
FAILED : 3
};

// GLTF_SPEC: Figure out correct mime types (https://github.com/KhronosGroup/glTF/issues/412)
var defaultModelAccept = 'model/vnd.gltf.binary,model/vnd.gltf+json,model/gltf.binary,model/gltf+json;q=0.8,application/json;q=0.2,*/*;q=0.01';
// GLTF_SPEC: Figure out correct mime types (https://github.com/KhronosGroup/glTF/issues/412 and https://github.com/KhronosGroup/glTF/issues/943)
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, last comment. Is this comment still needed? 1.0 and 2.0 specs are done and all mime types are known, right?

@emackey
Copy link
Contributor Author

emackey commented Jun 6, 2017

Good point about that comment. I wanted to keep the links, but I changed the rest of the wording to sound more final and less like a TODO.

@emackey
Copy link
Contributor Author

emackey commented Jun 9, 2017

This is ready.

@mramato
Copy link
Contributor

mramato commented Jun 9, 2017

Whoops, thought I merged this.

@mramato mramato merged commit 6dad54e into master Jun 9, 2017
@mramato mramato deleted the glb-mime-type branch June 9, 2017 14:56
@mramato
Copy link
Contributor

mramato commented Jun 9, 2017

Thanks @emackey!

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.

2 participants