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 always scroll to top issue on IE 7 #18

Closed
wants to merge 3 commits into from
Closed

Fix always scroll to top issue on IE 7 #18

wants to merge 3 commits into from

Conversation

chrisyip
Copy link

When the container is "window" and event is "scroll", will cause window to scroll back to top on IE 7.

The reason is

// settings.container = window
// settings.event = "scroll"
$(settings.container).trigger(settings.event);

works as the below code on IE 7:

`window.scrollTo(0, 0);``

Also, I cached window object to improve performance.

@tuupola
Copy link
Owner

tuupola commented Jan 21, 2012

I cannot merge this patch cleanly anymore since there have been quite many changes. But I am doing it manually. Great patch! Short scoping and caching the window object almost doubles the selector speed.

http://jsperf.com/lazyload-1-6-0
http://jsperf.com/lazyload-1-7-0

@ghost ghost assigned tuupola Jan 21, 2012
@tuupola
Copy link
Owner

tuupola commented Jan 21, 2012

Do you still experience the IE7 bug also with latest jQuery? To me it looks like it was fixed in 1.7.

http://bugs.jquery.com/ticket/6170

@chrisyip
Copy link
Author

Maybe latest jQuery fixed this issue, but for some frameworks, e.g. Drupal 6, you can't simply update jQuery to latest one, that's why I fix this issue, and this issue is easy to fix :-)

About the performance of copying window, on my Chrome 16.0.912.75 on Mac OS X 10.7.2, it's faster than don't copy.
In fact, I was just wanna pull request for the IE 7 issue, not the performance or something else :/
Coz I'm customizing it to fit our Drupal project.
Just Forget about it.

@tuupola
Copy link
Owner

tuupola commented Jan 21, 2012

That is interesting. I just tested Chrome 16.0.912.75 on Mac OS X 10.7.2 and I get around 4600 ops / second when copying and 7200 ops / second when caching.

Anyway, what version of jQuery Drupal 6 is using?

@chrisyip
Copy link
Author

Drupal 6 is using 1.2.6 for core library (WTF?!), with a third-part module, can be upgraded to 1.3.2.
In the past, I tried to upgrade to 1.5.2 (or lower), and lots of core features were broken.
Although I can load latest jQuery with $.noConflict() and initialize lazyload with it.

@ssorallen
Copy link

The issue affects even jQuery 1.7 as far as I can tell. Try scrolling to the bottom of http://www.ea.com/ and executing jQuery(window).trigger('scroll'), which is what happens in this plugin's resize listener by default. That call scrolls the page to the top.

@ssorallen
Copy link

Example: scroll to the bottom of your project page, http://www.appelsiini.net/projects/lazyload, and then resize the window. It scrolls to the top.

Edit: I'm on OS X, Chrome 17.0.963.56

@tuupola
Copy link
Owner

tuupola commented Feb 21, 2012

Actually project page has 1.6. Demo pages have 1.7.

http://www.appelsiini.net/projects/lazyload/enabled_fadein.html

But since many people are still using 1.6 and older I might consider including Chris' workaround for Chrome.

@ssorallen
Copy link

I noticed the 1.6/1.7 difference right after I wrote my comments. The workaround doesn't seem ideal, calling $window.scrollTo with the current coordinates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants