Skip to content

Conversation

@thisconnect
Copy link
Contributor

without that using the image later with webgl errors
DOMException: Failed to execute 'texImage2D' on 'WebGLRenderingContext': The image element contains cross-origin data, and may not be loaded.

  • Have you updated CHANGELOG.md?

Not sure if there is any reason why to not have this attribute. Please let me know

without that using the image later with webgl errors
 DOMException: Failed to execute 'texImage2D' on 'WebGLRenderingContext': The image element contains cross-origin data, and may not be loaded.
@thisconnect
Copy link
Contributor Author

sorry I dont know how to correctly write a test for this issue in node-canvas project. I mainly open the PR to discuss a possible change

@zbjornson
Copy link
Collaborator

Thanks for the PR! I'm not sure if this should be hard-coded; some users might want "use-credentials" and others might not want CORS at all. Maybe we could add an optional second argument that is an object with a crossOrigin property to let users set it. @LinusU or @chearon might have a preference.

@LinusU
Copy link
Collaborator

LinusU commented Apr 18, 2019

Maybe we could add an optional second argument that is an object with a crossOrigin property to let users set it.

This would be great, maybe even implement it as such:

- exports.loadImage = function (src) {
+ exports.loadImage = function (src, options) {
    return new Promise((resolve, reject) => {
-     const image = document.createElement('img')
+     const image = Object.assign(document.createElement('img'), options)

That way any properties that you can set on an HTMLImageElement can be passed 🙌

@zbjornson
Copy link
Collaborator

@thisconnect That looks good! Can you please update typings/index.d.ts to reflect that new option? (It can just be options?: any I think.)

@thisconnect
Copy link
Contributor Author

@zbjornson I added options?: any please check

@zbjornson zbjornson merged commit d407151 into Automattic:master Apr 24, 2019
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.

3 participants