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

Fix #12375: stop using document.body.scrollTop #12377

Closed
wants to merge 1 commit into from
Closed

Fix #12375: stop using document.body.scrollTop #12377

wants to merge 1 commit into from

Conversation

zlatanvasovic
Copy link
Contributor

Removes document.body.scrollTop as it's deprecated in the strict mode.

Fixes #12375.

/cc @fat

@cvrebert
Copy link
Collaborator

Have you tested that this doesn't break any of the non-Chrome browsers?

@zlatanvasovic
Copy link
Contributor Author

It works nice in Chrome, Firefox and Opera, not tested in IE and Safari.

2014-01-25 Chris Rebert [email protected]

Have you tested that this doesn't break any of the non-Chrome browsers?


Reply to this email directly or view it on GitHubhttps://github.com//pull/12377#issuecomment-33297794
.

Zlatan Vasović - ZDroid

@ghost
Copy link

ghost commented Jan 26, 2014

nice

@zlatanvasovic
Copy link
Contributor Author

Ref:
http://stackoverflow.com/questions/19635188/why-is-body-scrolltop-deprecated

2014-01-26 roymps [email protected]

nice


Reply to this email directly or view it on GitHubhttps://github.com//pull/12377#issuecomment-33313393
.

Zlatan Vasović - ZDroid

@cvrebert
Copy link
Collaborator

cvrebert commented Feb 9, 2014

Can't merge unless this gets tested in IE & Safari.

@cvrebert cvrebert added this to the v3.2.0 milestone Feb 9, 2014
@zlatanvasovic
Copy link
Contributor Author

Green Travis build (including Sauce) isn't enough? ;P

Can you test this on Safari, I don't have OS X?

@cvrebert
Copy link
Collaborator

cvrebert commented Feb 9, 2014

The Sauce build is a no-op unless the PR branch is in the twbs/bootstrap repo.

@zlatanvasovic
Copy link
Contributor Author

OK.

Note: this variant should theoretically work everywhere because it uses .scrollTop element method, but not on body. It's on documentElement now, which is supported by modern browers.

@XhmikosR
Copy link
Member

I will try this on IE.

@XhmikosR
Copy link
Member

Seems to work fine in IE8-10; I see the /javascript/#tooltips-examples as usual.

The PR needs rebasing BTW.

@fat
Copy link
Member

fat commented Mar 17, 2014

lgtm 👍

@cvrebert
Copy link
Collaborator

Seems to work on Safari too.

@cvrebert cvrebert closed this in ead17b7 Mar 17, 2014
@mdo mdo mentioned this pull request Mar 17, 2014
@zlatanvasovic zlatanvasovic deleted the strict branch March 17, 2014 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warning when I use Tooltips in Google Chrome
4 participants