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

sizes and srcset do not work on resize in Safari version 11.1.2 (13605.3.8) #31

Open
robots4life opened this issue Sep 11, 2018 · 6 comments
Labels

Comments

@robots4life
Copy link

robots4life commented Sep 11, 2018

This is not so much an issue with the seminal resimagelint but a rather sad fact I learned about Safari.

Following up from here , where I learned tons and thought I can now have responsive images working in the latest versions of the main browsers,kindly have a look at this demo that passes the resimagelint audit.

https://robots4life.github.io/respimagelint-debug-help/Safari/

When you look at it in Safari Version 11.1.2 (13605.3.8) and go from a horizontally narrow to a wide viewport the img never changes.
Only on reload of the page the correct img src is picked.
Here is a video.
https://github.com/robots4life/respimagelint-debug-help/blob/master/Safari/2018-09-11%2018-56-53.flv

All other current and main browsers manage to get this right.

We are in 2018.
When will Apple begin work on Safari so that also their users can enjoy responsive images like the rest of the web has been doing quite seamless for ages?
Do Apple users not resize the viewport?
Am I missing something?

Apparently Safari can handle srcset and sizes since 1st October 215.
https://caniuse.com/#feat=srcset
How come that more than 3 years later users are still expected to reload the page to be served the correct image for the current viewport?

If you have any hint or explanation or anything I could possibly change in the syntax to get at least the latest version of Safari to pick the right source on resize kindly let me know.

Have also tried with picturefill and lazysizes but nothing I do seems to want to make Safari pick the correct img on resize.

Would be kinda glad to hear "no there is nothing you can do" so I can give this a rest and point clients to https://bugs.webkit.org/show_bug.cgi?id=149899 so that they can take it up with Apple.
Also found this question on SO but that did not help.

Your thoughts on this are highly appreciated.

@ausi
Copy link
Owner

ausi commented Sep 11, 2018

You can add some JavaScript to trigger the Webkit engine after a browser window resize:

window.addEventListener('resize', function() {
	document.querySelectorAll('img[srcset]').forEach(function(img) {
		img.src += '';
	});
});

@ausi ausi added the question label Sep 11, 2018
@robots4life
Copy link
Author

robots4life commented Sep 11, 2018

Wow, nice one, thx heaps!
Still can't believe this has to be done in Safari to this day and age with responsive images using sizes and srcset.
Thank you tremendously for the suggestion & help there.

From what I understand the code simply removes the src attribute value on resize and the browser then, given the HTML source, has to fill in the next valid src given the srcset. I had the idea to also remove the src attribute but then also fill it again with JS.
Don't really understand how the browser is so smart to "refill" the src attribute with the correct value once the src attribute has been emptied by JS.

Safari 6.2.8 up to 9.1 gave me this error when I use your JS.

TypeError: undefined is not a function (evaluating document.querySelectorAll("img[srcset]").forEach(function(img) {
    img.src += "";
  })')

So I rewrote it to this below and now it works from Version 6.2.8 (8537.85.17.9.1) upwards to the current latest 11.1.2 (13605.3.8). Not looking below 6.2.8 because flexbox and flexwrap are not supported and that breaks too much in the layout.

window.addEventListener("resize", function() {
  var imgSrc = document.querySelectorAll("img[srcset]");
  for (var i = 0; i < imgSrc.length; i++) {
    imgSrc.src = "";
  }
});

For Safari below 9.1 I am including picturefill by feature detection of the picture tag, this by trying not to use Modernizr but to be as light as possible.

// [http://ryanmorr.com/determine-html5-element-support-in-javascript/](http://ryanmorr.com/determine-html5-element-support-in-javascript/)
function isElementSupported(tag) {
  var element = document.createElement(tag);
  return (
    Object.prototype.toString.call(element) !== "[object HTMLUnknownElement]"
  );
}

// https://stackoverflow.com/a/14174028
// Then add picturefill on the condition of the picture tag not being supported.
function require(file, callback) {
  var head = document.getElementsByTagName("head")[0];
  var pictureTag = "picture";
  if (isElementSupported(pictureTag) == false) {
    console.log(pictureTag + " supported : " + isElementSupported(pictureTag));
    // Load picturefill for browsers not supporting picture, sizes & srcset
    // https://stackoverflow.com/a/14174028
    var script1 = document.createElement("script");
    script1.type = "text/javascript";
    script1.src = "<?php get_template_directory_uri() . '/resources/vendor/js/min/picturefill.min.js'; ?>";
    //real browsers
    script1.onload = callback;
    //Internet explorer
    script1.onreadystatechange = function() {
      if (this.readyState == "complete") {
        callback();
      }
    };
    head.appendChild(script1);
  } else {
    console.log(pictureTag + " supported : " + isElementSupported(pictureTag));
  }

So all this checks out.
Finally I can also use responsive images in Safari.

However now I am facing the issue that the event listener for the Webkit trigger runs for every browser but I only need it in Safari. There ideally from the latest version downwards to at least 6.2.8.

Would be happy if this can be done with feature detection similar to the picture tag instead of with UA sniffing and the likes. Is there a simple JS way to determine 100% if the user is using a real Safari browser through a feature? I am sure there is and I am about to research that..

Again, thx heaps for the simple and quite gripping code above.

@ausi
Copy link
Owner

ausi commented Sep 11, 2018

From what I understand the code simply removes the src attribute value on resize

No. My code appends (+=) an empty string to the src attribute, which means it doesn’t really change it, but it triggers the browser engine to recalculate the image.

Looking at this issue again, I think it might be better (with a lower chance for unwanted side effects) to use the sizes attribute instead of the src attribute:

window.addEventListener('resize', function() {
  var sources = document.querySelectorAll('img[sizes],source[sizes]');
  for (var i = 0; i < sources.length; i++) {
    sources[i].sizes += '';
  }
});

Would be happy if this can be done with feature detection similar to the picture tag instead of with UA sniffing and the likes.

This bug is hard to detect AFAIK. I would probably either run it everywhere or not at all.

Again, thx heaps for the simple and quite gripping code above.

☺️

@robots4life
Copy link
Author

robots4life commented Sep 12, 2018

No. My code appends (+=) an empty string to the src attribute, which means it doesn’t really change it, but it triggers the browser engine to recalculate the image.

Excellent, thanks for the explanation, smart move there.

// Check for Safari
// https://stackoverflow.com/a/31732310/1010918
var isSafari =
  navigator.vendor &&
  navigator.vendor.indexOf("Apple") > -1 &&
  navigator.userAgent &&
  navigator.userAgent.indexOf("CriOS") == -1 &&
  navigator.userAgent.indexOf("FxiOS") == -1;

if (isSafari == true) {
  // Trigger Webkit engine after browser resize (Safari)
  // https://github.com/ausi/respimagelint/issues/31#issuecomment-420355534
  window.addEventListener("resize", function() {
    // For use with Lazysizes.js
    var imgSrc = document.querySelectorAll("img[data-src], source[data-src]");
    for (var i = 0; i < imgSrc.length; i++) {
      // console.log(imgSrc[i]);
      console.log("is Safari : " + isSafari);
      imgSrc[i].src += "";
    }
  });
}

This is what I am using now and Browserstack confirms I can trigger this only on Safari, including the iOS devices. Though on some old OS version Firefox also triggers this. I can live with that as opposed to running it everywhere. Also I changed the code a tiny bit to make it usable with lazysizes. Works perfect now.

Oh and where is this Safari bug referenced officially? Attending a talk on responsive images soon and would love to discuss the implications of this bug with the speaker, having a solid reference of this bug would be awesome.

Would you argue not having this isSafari check is safer than having it, meaning the implications for all browsers having to trigger their engine is remarkably small. I would imagine having to re-tirgger the engine on resize constantly could be bad for performance, no?

@ausi
Copy link
Owner

ausi commented Sep 12, 2018

Oh and where is this Safari bug referenced officially?

I think the link to the WebKit bug you posted looks like the best “official” reference: https://bugs.webkit.org/show_bug.cgi?id=149899

I would imagine having to re-tirgger the engine on resize constantly could be bad for performance, no?

I think most browsers don’t actually do more with this script then they do on resize anyway, especially if you use the sizes attribute instead of src. The wasted micro seconds wouldn’t hurt much IMO.

The problem with your detection is that you don’t catch all WebKit browsers but only Safari itself.

@robots4life
Copy link
Author

Not too happy about having the resize event listener for every Browser but at least like this the users on Apple / Safari devices & browsers can have responsive images. Using this in production and no feel-able performance hit so far, best would be if Apple & Safari could properly implement responsive images.

Thank you for your in depth help.

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

2 participants