fix #969 SynEvent used pageYOffset and pageXOffset on all version of IE. #969

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Member

fbasso commented Feb 4, 2014

These properties are not supported on IE <= 8 (http://www.quirksmode.org/dom/w3c_cssom.html#windowview), and returned undefined.

Now a fallback on scrollTop and scrollLeft has been done if these properties doesn't exist.

Member

fbasso commented Feb 4, 2014

Fix #898

Member

fbasso commented Feb 6, 2014

TemplateTestCase.js has been improved too in order to remove the test div instead of just hidding it at the end of the test, in order to avoid weird cases, for instance trying to focus an hidden element when several asynchronous tests are run in the same test (solve issue #664 ).

Member

fbasso commented Feb 6, 2014

fix #635

Member

fbasso commented Feb 6, 2014

fix #663

jakub-g self-assigned this Feb 7, 2014

@jakub-g jakub-g commented on an outdated diff Feb 7, 2014

src/aria/utils/SynEvents.js
@@ -2478,8 +2478,8 @@
box = el.getBoundingClientRect();
}
var o = {
- top: box.top + Aria.$window.pageYOffset - docElem.clientTop,
- left: box.left + Aria.$window.pageXOffset - docElem.clientLeft
+ top: box.top + (Aria.$window.pageYOffset || Aria.$window.document.body.scrollTop) - docElem.clientTop,
+ left: box.left + (Aria.$window.pageXOffset || Aria.$window.document.body.scrollLeft) - docElem.clientLeft
@jakub-g

jakub-g Feb 7, 2014

Collaborator

I've just done some random checks in Firefox, IE8, IE11, and it seems that body.scrollTop is not equivalent to pageYOffset (at least not always).

MDN suggests[1] to use sth like
window.pageYOffset || (document.documentElement || document.body.parentNode || document.body).scrollTop
for maximum portability so I'd stick to that (and same for pageXOffset)

(I'd put var document = Aria.$window.document before this declaration, and then use document inline for readability)

[1] https://developer.mozilla.org/en-US/docs/Web/API/Window.scrollY#Notes

@jakub-g

jakub-g Feb 7, 2014

Collaborator

Edit: well actually to be safe we should do precisely what they suggest, i.e.

var y = (window.pageYOffset !== undefined) ? window.pageYOffset : (document.documentElement || document.body.parentNode || document.body).scrollTop;

because pageYOffset can be 0. This shouldn't be much issue though because in all browsers I've checked, this implies also document.documentElement.scrollTop === 0 but we never know.

@fbasso fbasso added a commit to fbasso/ariatemplates that referenced this pull request Feb 7, 2014

@fbasso fbasso fix #969 SynEvent used pageYOffset and pageXOffset on all version of IE
These properties are not supported on IE <= 8 (http://www.quirksmode.org/dom/w3c_cssom.html#windowview),
and returned undefined.

Now a fallback on scrollTop and scrollLeft has been done if
these properties doesn't exist.

Close #664, close #635, close #663
4e6455d

@fbasso fbasso added a commit to fbasso/ariatemplates that referenced this pull request Feb 7, 2014

@fbasso fbasso fix #969 SynEvent used pageYOffset and pageXOffset on all version of IE
These properties are not supported on IE <= 8 (http://www.quirksmode.org/dom/w3c_cssom.html#windowview),
and returned undefined.

Now a fallback on scrollTop and scrollLeft has been done if
these properties doesn't exist.

Close #664, close #635, close #663
19202b3
Collaborator

jakub-g commented Feb 7, 2014

reviewed 19202b3 and it looks good for me

@fbasso fbasso fix #969 SynEvent used pageYOffset and pageXOffset on all version of IE
These properties are not supported on IE <= 8
(http://www.quirksmode.org/dom/w3c_cssom.html#windowview),
and returned undefined.

Now a fallback on scrollTop and scrollLeft has been done if
these properties doesn't exist.

Close #898, close #664, close #635, close #663
e5e512c

fbasso closed this in 2b72045 Feb 7, 2014

fbasso deleted the fbasso:synevent branch Feb 7, 2014

flongo added this to the 1.4.16 milestone Feb 13, 2014

@carlo-mr carlo-mr added a commit to carlo-mr/ariatemplates that referenced this pull request Mar 3, 2014

@fbasso @carlo-mr fbasso + carlo-mr fix #969 SynEvent used pageYOffset and pageXOffset on all version of IE
These properties are not supported on IE <= 8
(http://www.quirksmode.org/dom/w3c_cssom.html#windowview),
and returned undefined.

Now a fallback on scrollTop and scrollLeft has been done if
these properties doesn't exist.

Close #898, close #664, close #635, close #663
f527473
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment