useSrcset=true should also set src as fallback on img tag

[axnsan12]

In browsers that do not support srcset (e.g. IE11), ng-lazyload-image fails silently - onLoad is not even called with a false argument.

The proper way to handle this would be, in my opinion, to also set the src attribute to the largest image available in the srcset. This ensures that browsers without srcset support will at least load an image.

[tjoskar]

Hum.. weird that onLoad is never called. It should be called with true even if srcset is not supported (since the image should be loaded anyway). ng-lazyload-image does not know if srcset is supported or not so it will assume it is (if useSrcset=true) and try to load it any way. It will then pass the path to the loaded image to the browser and then unsubscribes to all events.

I do however agree that ng-lazyload-image should check if srcset is supported but I'm not sure how. The only way I know is to use CSS.supports but that is not supported in IE 🤦‍♂️.

[axnsan12]

I would assume that onLoad is not called because trying to set srcset raises an exception?

In any case, I would arguesrc should always be set, regardless of srcset support detection. Browsers that support srcset will ignore it anyways, so there is no downside.

[tjoskar]

... trying to set srcset raises an exception?

Html is backward compatible so we can always add new attributes and we can also add new propertys in dynamic objects in javascript. With that being said, IE is not like any other browser so you may be right but if it throws an error it should also set the error image (and css class):

ng-lazyload-image/src/lazyload-image.ts

Lines 127 to 131 in d63ac6a

	catchError(() => {
	setImageAndSourcesToError(element, errorImgPath, useSrcset);
	addCssClassName(element, cssClassNames.failed);
	return Observable.of(false);
	}),

and call onLoad with false.

So I'm not sure why onLoad is not called.

... would argue src should always be set ...

I agree and it should be quite easy to do, I guess we just need to change the following lines:

ng-lazyload-image/src/lazyload-image.ts

Lines 71 to 75 in d63ac6a

	if (useSrcset) {
	element.srcset = imagePath;
	} else {
	element.src = imagePath;
	}

to:

element.src = imagePath;
if (useSrcset) {
  element.srcset = imagePath;
}

Do you have any input @sapierens?

[sapierens]

I agree and it should be quite easy to do, I guess we just need to change the following lines:

It would work with <picture>, but wouldn't with <img> tags, since the srcset attribute has to be parsed to extract the correct image, e.g.:

<img srcset="img-320w.jpg 320w,
             img-480w.jpg 480w,
             img-800w.jpg 800w"
     sizes="(max-width: 320px) 280px,
            (max-width: 480px) 440px,
            800px">

Parsing this should be pretty easy (at least for selecting the largest image), but I'm not sure how much of an overhead it would add. But doing so would enable at least somewhat working lazy loading for browsers that don't support srcset attribute. Maybe it's worth looking into srcset polyfills to see how they're doing their detection and parsing.

[tjoskar]

Parsing this should be pretty easy [...]

From my understanding, only "width" (w) and "resolution" (x) are valid units so it should be fairly easy BUT am not a big fan of adding this kind of logic into this lib, especially if the browsers start to support more units.

I beleve it is better to make it possible to pass a custom setImage method so the user can extend the libary and write custom logic to set the image.

Which is discussed in #304, #322 and #323

[sapierens]

BUT am not a big fan of adding this kind of logic into this lib

I agree, trying to parse responsive images shouldn't be the responsibility of this library.

The problem presented here actually goes back to my initial PR #231 (see under Things under question section). If we opted for [lazyLoad] and [lazyLoadSrcset] options, then users could set the fallback src image themselves. Which now makes more sense with this issue present.
But to implement it now would be a pretty breaking change for existing users who use responsive images.

I also agree that the ability to pass custom setImage etc. methods should be implemented in any case.

[tjoskar]

This should be solved now with hooks (loadImage and setLoadedImage).