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

newer (e.g. 31) Chromes complain about what to scroll #61

Closed
szepeviktor opened this issue Jan 20, 2014 · 25 comments
Closed

newer (e.g. 31) Chromes complain about what to scroll #61

szepeviktor opened this issue Jan 20, 2014 · 25 comments
Assignees
Labels

Comments

@szepeviktor
Copy link
Contributor

body.scrollTop is deprecated in strict mode.
Please use 'documentElement.scrollTop' if in strict mode and 'body.scrollTop' only if in quirks mode.
jquery.scrollTo-amd.min.js:1

Please distinguish between old and new webkits.

(and you may use "use strict";)

@flesler
Copy link
Owner

flesler commented Jan 20, 2014

That's a warning that pops in the console?

@szepeviktor
Copy link
Contributor Author

ofcourse

@nitriques
Copy link

@szepeviktor
Copy link
Contributor Author

Then it is OK now.
I do not care about the console.

@nitriques
Copy link

I do not care about the console.

You should.

@tmorehouse
Copy link

I've seen other similar error from jQuery itself in the console log. Right now they are just warnings. so nothing major to to worry about.

@szepeviktor
Copy link
Contributor Author

I mean this console message.
Or should this webkit condition be deleted?
https://github.com/flesler/jquery.scrollTo/blob/master/jquery.scrollTo.js#L48

@tmorehouse
Copy link

maybe change it to:

return (document.body.scrollTop) ? document.body : document.documentElement;

@szepeviktor
Copy link
Contributor Author

That may cause an error.
return (document.documentElement.scrollTop) ? document.documentElement : document.body;

@tmorehouse
Copy link

Maybe:

return (document.documentElement && document.documentElement.scrollTop) ? 
    document.documentElement : document.body;

Mithgol referenced this issue in flesler/jquery.localScroll Jan 22, 2014
@flesler
Copy link
Owner

flesler commented Jan 24, 2014

Let's keep this open, I'm afraid nothing might work once Chrome 33 is released

@flesler
Copy link
Owner

flesler commented Jan 24, 2014

Maybe $(window).scrollTop() will do? not sure, need to do some research.
I appreciate if you guys help out.

@nitriques
Copy link

@flesler

Maybe $(window).scrollTop()

I ALWAYS use that. Never body, nor html. If it's on part of the document, it's always a div.

@szepeviktor
Copy link
Contributor Author

jQuery 1.10.2

// Create scrollLeft and scrollTop methods
jQuery.each( {scrollLeft: "pageXOffset", scrollTop: "pageYOffset"}, function( method, prop ) {
    var top = /Y/.test( prop );

    jQuery.fn[ method ] = function( val ) {
        return jQuery.access( this, function( elem, method, val ) {
            var win = getWindow( elem );

            if ( val === undefined ) {
                return win ? (prop in win) ? win[ prop ] :
                    win.document.documentElement[ method ] :
                    elem[ method ];
            }

            if ( win ) {
                win.scrollTo(
                    !top ? val : jQuery( win ).scrollLeft(),
                    top ? val : jQuery( win ).scrollTop()
                );

            } else {
                elem[ method ] = val;
            }
        }, method, val, arguments.length, null );
    };
});

function getWindow( elem ) {
    return jQuery.isWindow( elem ) ?
        elem :
        elem.nodeType === 9 ?
            elem.defaultView || elem.parentWindow :
            false;
}

@flesler
Copy link
Owner

flesler commented Jan 24, 2014

I think you can see the warning by using vanilla jQuery. Not sure the core implementation is the solution.

@tmorehouse
Copy link

I see jQuery throwing the same error once and a while, just by itself.

@tmorehouse
Copy link

http://dom.spec.whatwg.org/#concept-document-quirks

you could maybe do a test on document.compatMode. If it exists and has a value of CSS1Compat then scroll the documentElement, otherwise revert to scrolling the body element.

The "strict mode" chrome is referring to is not use strict, but rather the rendering mode of the browser (based on the DOCTYPE)

change this:

return /webkit/i.test(navigator.userAgent) || doc.compatMode == 'BackCompat' ?
    doc.body : 
    doc.documentElement;

to this maybe:

return doc.compatMode && doc.compatMode == 'CSS1Compat' ?
    doc.documentElement :
    doc.body;

@flesler
Copy link
Owner

flesler commented Jan 24, 2014

I'll try this fix when I get home

facebook/react#643

Also I'll take from jQuery's code, the one Viktor posted.

On Fri, Jan 24, 2014 at 12:54 PM, Troy Morehouse
notifications@github.comwrote:

http://dom.spec.whatwg.org/#concept-document-quirks

you could maybe do a test on document.compatMode. If it exists and has a
value of CSS1Compat then scroll the documentElement, otherwise revert to
scrolling the body element.

The "strict mode" chrome is referring to is not use strict, but rather
the rendering mode of the browser (based on the DOCTYPE)


Reply to this email directly or view it on GitHubhttps://github.com//issues/61#issuecomment-33234233
.

Ariel Flesler

@flesler
Copy link
Owner

flesler commented Jan 25, 2014

Ok, I've been doing some experiments on Windows 7.
Updated the tests to use the latest version of jquery, you can try them yourself.
The results were unexpected.

Chrome 32 shows a warning when on compatibility mode as expected:

Chrome 33 beta actually showed no warning and worked with the current version of scrollTo just fine:

I tried using the documentElement on all cases and then on compat mode, Chrome 33 didn't work at all.
So the lesson is, Chrome 33 beta only works with the body which is the way it is right now.

@szepeviktor @nitriques @tmorehouse @Mithgol
I encourage you to try it yourself on other platforms and browsers let me know what you get.

@nitriques
Copy link

Thanks @flesler
I will test tomorrow!

@flesler
Copy link
Owner

flesler commented Jan 29, 2014

How did it go @nitriques ?
Can you guys take a look too? @szepeviktor @tmorehouse @Mithgol

@nitriques
Copy link

Chrome 32 still complains in WinMaxY-compat.html.
But Chrome 34 (canary) does not.

@flesler
Copy link
Owner

flesler commented Jan 29, 2014

Good, same as me. Thanks.

@nitriques
Copy link

You're welcome :)

@ghost ghost assigned flesler Feb 2, 2014
@flesler
Copy link
Owner

flesler commented Feb 2, 2014

Well, closing this. Reopen if something else related comes up

@flesler flesler closed this as completed Feb 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants