Added workaround for window.scrollX compat. #1941
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
window.scrollX/Y is not available in IE11. As far as specifications go, is currently only specified in draft (http://dev.w3.org/csswg/cssom-view/#refsCSSOM). Falling back to window.pageXOffset seems like a good workaround. On a related note; my Emscriptified project runs on IE11 although performance is very poor (mostly due to Internet Explorer itself, I think). It's pretty finicky about the shaders, as they introduced an extra set of requirements. (inout/in/out keywords not supported, can't construct mat3 from mat4, etc).
|
Looks good, please just add a little comment explaining it is a fallback in case the browser does not support that feature. Link to your project? Curious to see performance on IE11 myself. I've heard it's WebGL implementation still has issues, hopefully those will be fixed in IE12... |
Added comments to fix as per @kripken's request. Added an assert too (guarded by ASSERTIONS define)
|
There we go, added an assert too. My project should be up shortly, I'll drop a note here when it is :D As for IE11's performance, I forgot I had GPU compositing turned off which caused a severe performance hit, d'oh. |
Forgot I had to explicitly check the type of variable type to see if it's undefined.
| //(see: http://www.w3.org/TR/2013/WD-cssom-view-20131217/) | ||
| var scrollX = ((typeof window.scrollX !== 'undefined') ? window.scrollX : window.pageXOffset); | ||
| var scrollY = ((typeof window.scrollY !== 'undefined') ? window.scrollY : window.pageYOffset); | ||
| #if ASSERTIONS |
There was a problem hiding this comment.
please do not indent lines starting with # (see other examples in our source files)
|
Done. |
kripken
added a commit
that referenced
this pull request
Jan 4, 2014
Added workaround for window.scrollX compat.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
window.scrollX/Y is not available in IE11. As far as specifications go, is currently only specified in draft (http://dev.w3.org/csswg/cssom-view/#refsCSSOM). Falling back to
window.pageXOffset seems like a good workaround.
On a related note; my Emscriptified project runs on IE11 although performance is very poor (mostly due to Internet Explorer itself, I think). It's pretty finicky about the shaders, as they introduced an extra set of requirements. (inout/in/out keywords not supported, can't construct mat3 from mat4, etc).