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

window.pageYOffset returns zero safari 8.x #305

Closed
gregh3269 opened this issue Feb 14, 2019 · 14 comments
Closed

window.pageYOffset returns zero safari 8.x #305

gregh3269 opened this issue Feb 14, 2019 · 14 comments

Comments

@gregh3269
Copy link

gregh3269 commented Feb 14, 2019

Are you reporting a bug or requesting a feature?

If it's a bug, it would be very useful to me if you answered the following questions.
Your answers will help me understand what's happening and enable me to reproduce the bug.

  • What is happening?
  • What is the expected behaviour?
  • Which version of LazyLoad are you using?
  • In which browser(s) and browser version(s) are you experiencing the problem?
  • Could you provide the HTML, CSS and JS code you use for your images?
  • What are the steps to reproduce the problem?

Thank you.

@gregh3269
Copy link
Author

Hello,
For version 8.x on safari (<10) I get window.pageYOffset = 0 so the scroll down does initialise the images. The initial viewport works OK and shows the images.

A simple modification would be to replace window.pageYOffset with getPageYOffset() below :

var getPageYOffset = function getTopOffset(element) {
return window.pageYOffset || document.documentElement.scrollTop || document.body.scrollTop || 0;
};

Think this is a known workaround, where it checks the know variations for the scroll/screen position.

Cheers Greg

@verlok
Copy link
Owner

verlok commented Feb 14, 2019

Hi @gregh3269

could you add some context please? There was a list of questions in the issue template, but you didn't answer any.

Specifically, I miss the following information:

  • Could you provide the HTML, CSS and JS code you use for your images?
  • What are the steps to reproduce the problem?

Thanks

@gregh3269
Copy link
Author

gregh3269 commented Feb 15, 2019

OK, sorry.
8.x (8.17.0) works great, this is for older phone support webkit/safari.

When I load the page (on the phone) it renders the images within the threshold/screen, but when I scroll down it does not initialise the rest of the images.

I am using the code from the conditional conditional_load.html on 8.17.0.

It is an older phone webkit/safari maybe v6/7? (Later then IE9)

On debugging, it does not fire any of the class_loading, class_loaded etc methods, which I traced back to window.pageYOffset always returning 0 in function isBelowViewport(..) (others also)

var isBelowViewport = function isBelowViewport(element, container, threshold) {
		var fold = container === window ? window.innerHeight + window.pageYOffset : getTopOffset(container) + container.offsetHeight;
		return fold <= getTopOffset(element) - threshold;
	};

The suggested patch still uses the original window.pageYOffset, but if it returns 0, then checks other know methods of calculating the y position.

I have tried quite a few loaders, but none wanting to support older devices. I have patched this locally but it would be great if I could use jsdelivr.

Cheers Greg

@gregh3269
Copy link
Author

gregh3269 commented Feb 15, 2019

.. typo on patch :(

var getPageYOffset = function getPageYOffset() {
		return window.pageYOffset || document.documentElement.scrollTop || document.body.scrollTop || 0;
	};

@verlok
Copy link
Owner

verlok commented Feb 16, 2019

Thanks @gregh3269,

it sounds like a good patch, I will try this on my plugin later today and tell you whether or not I noticed regressions on other browsers. It shouldn't.

If you could kindly post here the entire patched lazyload.viewport.js file you can find on the support/8.x branch it would make this easier for me (I recently became a father for the 2nd time and I have less time for this open source project).

Thanks!

@verlok
Copy link
Owner

verlok commented Feb 16, 2019

Hi again,

I'm reading the documentation for pageYOffset and scrollTop and I'm afraid that the patch you're suggesting might cause bugs on corner cases, like a document containing different scrolling areas or different iframes.

I prefer not to touch such a sensitive aspect version 8.x of LazyLoad because it's quite stable and it's been for years. Furthermore, as Safari is going to support IntersectionObserver very soon, I'm going to deprecate the usage of this version in favor of version 10 only + a the official IntersectionObserver polyfill where needed (on IE and older Safari versions).

See #306 for more information.

@verlok
Copy link
Owner

verlok commented Feb 16, 2019

By the way @gregh3269 are you sure your images are inside the main document and not in a scrolling container e.g. overflow: scroll element?

Edit: pretty obviously the answer is that you're sure, I'm just asking.

@gregh3269
Copy link
Author

gregh3269 commented Feb 17, 2019

It does seem a can of worms.

With code below, it loads and shows the images, but I think its loading all the images as a default as I cannot see the delay being applied when I scroll up.

<script type="text/javascript" src="mysite/vanilla-lazyload/10.20.0/lazyload.js"></script>
<script src="http://polyfill.io/v2/polyfill.min.js?features=es5,IntersectionObserver,getComputedStyle"></script>
var myLazyLoad = new LazyLoad({
      elements_selector: ".lazyload",load_delay: 500
});

The methods I ended up applying the change to were isBelowViewport(..) and isAboveViewport(..). For some reason in getTopOffset(..) it breaks it.

Cheers Greg

@verlok
Copy link
Owner

verlok commented Feb 17, 2019

Hi @gregh3269,

the reason it loaded all the images at once in that case is that you placed the polyfill after LazyLoad, whereas you should place it before it.

The order is very important in this case because when LazyLoad v.10 is executed, it checks whether or not IntersectionObserver is present, and if it isn’t it loads all the images at once (as a fallback).

If you switched the order of the scripts, it should working a champ.

Let me know

@gregh3269
Copy link
Author

gregh3269 commented Feb 18, 2019

OK, I have managed to get the poly fill working v3 (not v2) in the url. But it still loads all the images at the start. I have added a trace to callback_set

var myLazyLoad = new LazyLoad({
          elements_selector: ".lazyload",load_delay: 500,
          callback_set: function(el) {
            $("#test").append(" callback_set ");
            console.log("callback_set");
          }
 });

I get callback_set for the number of images at load. So there is no delay.

from the lazyload.js:
supportsIntersectionObserver = true and runningOnBrowser = true;

I will try and debug more, but any pointers would be a help.

Cheers Greg

@gregh3269
Copy link
Author

OK, giving up, poly fill seems not to be able to calculate intersections and always return true on phone. :(
Cheers Greg

@verlok
Copy link
Owner

verlok commented Feb 23, 2019

Hey @gregh3269,

if you have an environment where I can test it, or you could replicate the issue on codepen, I can take a look and help you.

@gregh3269
Copy link
Author

Thanks, but since polyfill.io supports >= Safari 9 (2016), which I would have to mod to make it work with the phone, its simpler to roll my own version of 8 which works well.

Cheers

@verlok
Copy link
Owner

verlok commented Feb 25, 2019

Ok. Glad you solved in some way.

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

No branches or pull requests

2 participants