search

flesler / jquery.scrollTo

Lightweight, cross-browser and highly customizable animated scrolling with jQuery
http://demos.flesler.com/jquery/scrollTo/
MIT License
3.69k stars 1.03k forks source link

Google Chrome 40 breaks document scrolling #101

Closed LexLythius closed 9 years ago

LexLythius commented 9 years ago

Google Chrome 40 (beta in Windows 7, 64-bit at least) breaks scrolling because the scrollable element is no longer BODY but HTML. As a result, all tests in compat mode fail to scroll, whereas tests in quirks mode work fine. Everything worked fine in Chrome 39.0 though.

I've filed a bug report for this https://code.google.com/p/chromium/issues/detail?id=440504

Meanwhile, this ugly fix in $.fn.scrollTo worked for me:

<                   $elem = $(elem),

---
>                   $elem = $($.inArray(elem.tagName, [ 'HTML', 'BODY' ]) ? 'html,body' : elem),

but as a general solution it sucks, since it will trigger callbacks twice.

This is related, but not sure if the same as, #85 and #92

flesler commented 9 years ago

Ok I'm following the issue you created on their end, let's see what they respond

LexLythius commented 9 years ago

Great news, folks! The Chrome guys will not fix this for the time being, so the soon-to-be-released Chrome 40 breaks your plugin: https://code.google.com/p/chromium/issues/detail?id=440504#c4

flesler commented 9 years ago

Same thing happened before, I think twice. In the end the released version worked fine (god knows why). I'll only change the code if the actual released version breaks. In case you notice that before me, post in here right away if Chrome 40 breaks scrollTo.

Thanks

npetrovski commented 9 years ago

Chrome Version 40.0.2214.93 m - Still doesn't work unless I apply the change suggested by Lex.

flesler commented 9 years ago

@npetrovski Hi I'm actually using that same version of Chrome (latest). I ran the tests at http://flesler.github.io/jquery.scrollTo/tests/ and they all work fine for me on Win 7 (except last 2 if run on file:// ofc).

Can you run the tests and see if any of them doesn't scroll? do you have any non-default setting on your Chrome? If any test fails, copy the userAgent that appears in the test in here.

Thanks

npetrovski commented 9 years ago

Just ran them all - seems only the quirks mode works fine. The compact mode has the same problem as it was mentioned.

Chrome Version 40.0.2214.93 m; Windows 7 Enterprise 64 bit

npetrovski commented 9 years ago

Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.93 Safari/537.36 document.compatMode is "CSS1Compat" scrolling the BODY

Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.93 Safari/537.36 document.compatMode is "BackCompat" scrolling the BODY

No custom settings.

npetrovski commented 9 years ago

Tested also on: IE11 Version 11.0.9600.17420 - Works fine (all of them) Firefox 35.0 - Works fine (all tests)

flesler commented 9 years ago

Ok, added 2 more tests for scrolling a div with overflow. Also pushes to gh-pages so people can try the tests at http://flesler.github.io/jquery.scrollTo/tests/ It's strange, we have same browser version and both on Win 7. Will try to dig deeper, any insight is appreciated

flesler commented 9 years ago

When you run one of the compat mode you get this?

document.compatMode is "CSS1Compat"
scrolling the BODY
flesler commented 9 years ago

Just ran them on the same Chrome version on a Mac running Yosemite, all worked correctly.

npetrovski commented 9 years ago

http://flesler.github.io/jquery.scrollTo/tests/WinMaxY-compat.html - NO http://flesler.github.io/jquery.scrollTo/tests/WinMaxY-quirks.html - WORKS http://flesler.github.io/jquery.scrollTo/tests/ElemMaxY-compat.html - WORKS <---------- http://flesler.github.io/jquery.scrollTo/tests/ElemMaxY-quirks.html - WORKS http://flesler.github.io/jquery.scrollTo/tests/WinMaxY-with-iframe-compat.html - NO http://flesler.github.io/jquery.scrollTo/tests/WinMaxY-with-iframe-quirks.html - WORKS http://flesler.github.io/jquery.scrollTo/tests/WinMaxY-to-iframe-compat.html - NO http://flesler.github.io/jquery.scrollTo/tests/WinMaxY-to-iframe-quirks.html - WORKS

flesler commented 9 years ago

quoting @mgravell on https://github.com/flesler/jquery.scrollTo/issues/85#issuecomment-52613867

Any chance that chrome://flags/#enable-experimental-web-platform-features is enabled? There is a problem with this (will post a separate issue)
npetrovski commented 9 years ago

Nope - "Enable experimental Web Platform features" is disabled.

The difference I see now is that the "elem" variable @ line 78 dumps "html" element on Firefox and "body" element on Chrome - could it be a jQuery selector issue?.

LexLythius commented 9 years ago

@npetrovski Yes, this is a Chrome-specific issue. Firefox has kept the same scrolling element all along.

LexLythius commented 9 years ago

I've just run the tests again:

Updated Chrome version 40.0.2214.93 (64-bit) on Ubuntu 14.04 (Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.93 Safari/537.36):

ALL tests work fine.

Beta for upcoming Chrome 41 on Windows 7 x64 (Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2272.16]:

http://flesler.github.io/jquery.scrollTo/tests/WinMaxY-compat.html FAILS http://flesler.github.io/jquery.scrollTo/tests/WinMaxY-quirks.html WORKS http://flesler.github.io/jquery.scrollTo/tests/ElemMaxY-compat.html WORKS http://flesler.github.io/jquery.scrollTo/tests/ElemMaxY-quirks.html WORKS http://flesler.github.io/jquery.scrollTo/tests/WinMaxY-with-iframe-compat.html FAILS http://flesler.github.io/jquery.scrollTo/tests/WinMaxY-with-iframe-quirks.html WORKS http://flesler.github.io/jquery.scrollTo/tests/WinMaxY-to-iframe-compat.html FAILS http://flesler.github.io/jquery.scrollTo/tests/WinMaxY-to-iframe-quirks.html WORKS

These are the same results @npetrovski got.

LexLythius commented 9 years ago

On a side note, this breaks because you lack a way for feature detection here so you fall back to browser sniffing.

If Chrome intends to actually switch from HTML to BODY as main scrollable element, it would seem reasonable to place a feature request for a property that exposes this.

LexLythius commented 9 years ago

@npetrovski Do you know if your patch works fine with Safari and Opera as well? (can't test here).

npetrovski commented 9 years ago

@LexLythius nope - the fix I just suggested was deleted since the quirks mode in FF became broken. Will try something else.

LexLythius commented 9 years ago

Just tested Chrome 40.0.2214.93 on Windows 8.1 Enterprise (Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.93 Safari/537.36)

All tests work fine.

Maybe this is some change in development versions that didn't make it yet into the main release branch.

flesler commented 9 years ago

That patch would break it for everyone that is not experiencing the problem, like @LexLythius and me. My Chrome won't scroll if the HTML is used, it only works on BODY like all previous versions.

@LexLythius I wouldn't worry about future versions, it's always the same and in the end it works (except for some reason for @npetrovski and probably some other people)

flesler commented 9 years ago

Added a new branch called window-scroll. Thought I was going to overhaul this to something entirely based on support detection, but didn't quite work for non-Chrome browsers.

The new implementation (1.5.0-beta) relies on "support detection" on Chrome and compatMode for all other browsers. The tests on githubio use 1.5.0-beta, please go re-test them with as many browsers as possible.

Thanks

flesler commented 9 years ago

Just tested iOS Chrome and the userAgent doesn't include the word Chrome. Made it so it now handles Desktop Chrome, iOS Safari and iOS Chrome. All 3 now work correctly (I think iOS never worked!). Tested on Safari Mac and it works with the non-Chrome approach after all.

Updated the tests with these changes. It'd be great to test an Android, I recall the userAgent including AppleWebkit it might work or not.

npetrovski commented 9 years ago

For 1.5.0-beta - confirmed to work on:

IE 11.0.9600.17420 Desktop Chrome 40.0.2214.93 Firefox 35.0 Safari for windows 5.1.7 (7534.57.2)

pix666 commented 9 years ago

Both versions (1.4.14 and 1.5.1-beta) are not working on Chrome 40.0.2214.115 m.

Version 1.5.1-beta contents this:

// Changed, then it works
if (++elem.scrollTop === 1) {
...

That's not true for me. By default, document.body.scrollTop is 0. Changing it's value not leads to scrolling.

P.S. Tests results are the same as @npetrovski and @LexLythius got.

P.P.S. Win7; Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.115 Safari/537.36 document.compatMode is "CSS1Compat"

pix666 commented 9 years ago

That's chrome console shows:

> document.body.scrollTop
< 0

> ++document.body.scrollTop
< 1

> // Now, check again
> document.body.scrollTop
< 0
flesler commented 9 years ago

@pix666 thank you for chiming in, I'm reluctant to merge into master without many people that can verify it works. It seems your works as mine. Have you checked the chrome flags mentioned on this comment?

I went ahead and changed the fix based on this and it might actually be a key discovery. Tested on Firefox and IE and the same rule applies, so maybe, what I added as "Chrome hack" could be a reliable approach for all browsers.

In case this doesn't work in all cases, I came up with another (longer) solution, if one adds a window scroll handler, it only triggers when it actually scrolls. This one is simpler so we'll go with it for now.

Can you retry the tests?

pix666 commented 9 years ago

Have you checked the chrome flags mentioned on this comment?

"Enable experimental Web Platform features" is disabled.

I tested Chrome again. Much of tests shows alert "FAIL: scrollable must always return exactly 1 element". But if I reduce the size of the browser until document becomes scrollable - all tests work fine. When document have no scrollbar, document.body.scrollTop and document.documentElement.scrollTop are unchangeable. So, $(window)._scrollable() returns empty list.

Maybe should check whether element is basically scrollable?

flesler commented 9 years ago

Oh I understand now. I just made a slight change so that _scrollable() can never return 0 elements. I deployed it to the tests (same version).

pix666 commented 9 years ago

Test results for some browsers:

Chrome 40.0.2214.115m - ALL OK Firefox 35.0.1 - ALL OK Firefox 36.0 - ALL OK IE 11 - ALL OK Opera 27.0.1689 - ALL OK Safari 5.1.7 (for Windows) - ALL OK Android (Default Browser) - ALL OK Android (CM Browser) - ALL OK

flesler commented 9 years ago

Great! Adding my tests from Windows 7 and iPhone 4S iOS 8: IE 10 - ALL OK Firefox 35.0.1 - ALL OK Chrome 40.0.2214.73 (iPhone) - NONE OK Chrome 40.0.2214.115 (Windows) - ALL OK Safari (iPhone) - ALL OK

I need to look into Chrome iOS, though I don't think it was working before and it's not the biggest vendor so we could release as it is and then I hotfix Chrome iOS with a patch version.

flesler commented 9 years ago

Adding tests from a Mac with Yosemite 10.10.1

Chrome 41.0.2272.76 - ALL OK Safari 8.0 (10600.1.25.1) - ALL OK

markvantilburg commented 9 years ago

Windows 8.1 Firefox 36 Mozilla/5.0 (Windows NT 6.3; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0 - ALL OK

Chrome 41 Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2272.76 Safari/537.36 - ALL OK

Internet explorer 11 Mozilla/5.0 (Windows NT 6.3; WOW64; Trident/7.0; .NET4.0E; .NET4.0C; .NET CLR 3.5.30729; .NET CLR 2.0.50727; .NET CLR 3.0.30729; rv:11.0) like Gecko - ALL OK

markvantilburg commented 9 years ago

Windows phone 8.1 (IE - desktop mode) ALL OK

Windows phone 8.1 (IE - mobile version) ALL OK

user agents: ie_mobile

flesler commented 9 years ago

Hi all, I found an interesting alternative approach mentioned on StackOverflow and decided to try it as well. It delegates much of the getting and setting scroll positions logic to jQuery's core.

It is already implemented on the window-scroll-alt branch.I decided that if this alternative was to prevail, it should be 2.0.0 rather than 1.5.0 because it's even less backwards-compatible than this option and it contains a highly requested and powerful feature.

Could you please run the tests again on the same devices but accessing this tests? I kept the other ones at the original URL for comparison. If you comment with results, please do that on #107.

npetrovski commented 9 years ago

For branch window-scroll-alt :

FF 36.0 - OK IE 11.0.9600.17633 - OK Chrome 40.0.2214.93 - OK Safari for PC 5.1.7 (75.57.2) - OK Android FF 37.0 - OK Android Chrome 40.0.2214.109 - OK

Congratz - wrap it in 2.0.0 :-)

flesler commented 9 years ago

I merged 2.0.0 to master via #109 and this issue should be fixed. If you have any issue after the update, first check this link, If your problem is not solved then go ahead and report the issue.