Skip to content

Improved image creation #334

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

Merged
merged 1 commit into from
Mar 19, 2015
Merged

Improved image creation #334

merged 1 commit into from
Mar 19, 2015

Conversation

bud-mo
Copy link
Contributor

@bud-mo bud-mo commented Mar 19, 2015

new Image() creation can trigger a bug into an older version of Chrome.
I suggest the 'more compliant' way to create a node into the dom.

http://stackoverflow.com/questions/15469763/an-attempt-was-made-to-reference-a-node-in-a-context-where-it-does-not-exist

`new Image()` creation can trigger a bug into an older version of Chrome.
I suggest the 'more compliant' way to create a node into the dom.

http://stackoverflow.com/questions/15469763/an-attempt-was-made-to-reference-a-node-in-a-context-where-it-does-not-exist
@mattrobenolt
Copy link
Contributor

This seems legit.

From all of my understanding, I'm not sure new Image() in this context could introduce a bug since we never append the element to the DOM.

But with that said, all sources are saying that new Image() is just super old and to just use document.createElement('img') instead, so that works for me.

For posterity, this is my best finding on this: https://stackoverflow.com/questions/6241716/is-there-a-difference-between-new-image-and-document-createelementimg

Thanks! 🍰 ✨

mattrobenolt added a commit that referenced this pull request Mar 19, 2015
Improved image creation
@mattrobenolt mattrobenolt merged commit 5db3913 into getsentry:master Mar 19, 2015
@mattrobenolt
Copy link
Contributor

Crap, this did break tests though. :/ I have no idea why Travis isn't testing pull requests or I would have caught this before merging.

@bud-mo
Copy link
Contributor Author

bud-mo commented Mar 19, 2015

I have edited the file on GitHub, I was not able to do some tests.
Can I keep the cake? 😇

mattrobenolt added a commit that referenced this pull request Mar 19, 2015
I have no idea how to get sinon to actually stub
`document.createElement(‘img’)` and every attempt causes Phantom to
stop responding.

This was the easiest way out. I’m sorry.

This is a test backfilled for #334
@mattrobenolt
Copy link
Contributor

You still get cake.

matghaleb pushed a commit to matghaleb/raven-js that referenced this pull request Sep 9, 2015
I have no idea how to get sinon to actually stub
`document.createElement(‘img’)` and every attempt causes Phantom to
stop responding.

This was the easiest way out. I’m sorry.

This is a test backfilled for getsentry#334
kamilogorek pushed a commit that referenced this pull request Jun 12, 2018
Version 2.1.0
- Truncate long lines in surrounding source to avoid sending large amounts of minified code [See #329]
- Refactor automatic breadcrumb instrumentation of modules to accommodate compilation tools [See #322]
- Testing for Node 8 [See #328]
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