Wp-rocket: Add <noscript> tags for lazyloaded <picture>

[vmanthos]

Currently we are adding <noscript> tags for <img>s.
These are used by some crawlers, e.g. Ahrefs, to discover images.

We could implement this for the <picture> tag too.

Slack discussion: https://wp-media.slack.com/archives/C43T1AYMQ/p1582724043031900
Related ticket: https://secure.helpscout.net/conversation/1089441428/147251?folderId=2135277

[GeekPress]

@hellofromtonya @arunbasillal Do you have any thoughts about that? Should we do it?

[hellofromtonya]

@GeekPress LazyLoad requires JS. But we shouldn't assume that JavaScript is available for every website. To deal with JS availability, we add the <noscript> as a fallback. That fallback ensures that the element renders.

Here, the conversation is about the crawler bots. But I think it's bigger than that.

👉 By default, we should have <noscript> for each element that we target with LazyLoad. In that way, we ensure there's a callback in cases where there's no JS available.

Here's more information on JS availability strategies:

• Google Developers
• MDN

👉 For the <picture> element, I think we should add <noscript> as a fallback.

[arunbasillal]

By default, we should have

Following Tonya's advise, when we implement this, shall we implement this for all tags including picture tags. This way we can cover all cases.

[hellofromtonya]

Following Tonya's advise, when we implement this, shall we implement this for all tags including picture tags. This way we can cover all cases

@arunbasillal Yes, we should by default. Everything being LazyLoaded should have a <noscript> in it.

[GeekPress]

Scheduled for 3.6.2.

Thanks everybody for your feedbacks.

[crystinutzaa]

Reproduce the issue ✅
Identified the issue on my localhost.

Identify the root cause ✅
Root cause is related to missing noscript for: picture tag and lazyload background images.

Scope a solution ✅
Add noscript for picture and lazyload background images.
https://github.com/wp-media/rocket-lazyload-common/blob/fde493535ca389882484327af32c99c839d2527f/src/Image.php#L111
https://github.com/wp-media/rocket-lazyload-common/blob/fde493535ca389882484327af32c99c839d2527f/src/Image.php#L53

Effort ✅
Effort [S] , I think Unit & Integration tests will need almost all the effort.

@hellofromtonya & @Tabrisrp should we add the noscript also for background images?
Noscript will be displayed only there is no javascript active, however for background images how we prevent to display both divs? I am missing something?

[Tabrisrp]

Not a fan of the <noscript> for background images, the element could be wrapping a lot of other ones, and we would have to include everything in it, I don't think that makes sense.

I would suggest to start with the noscript for picture specifically, and look into the rest if we ever get users requests for it.

[crystinutzaa]

@arunbasillal Yes, we should by default. Everything being LazyLoaded should have a <noscript> in it.

Based on the previous message ^^^ I was adding the background images no-script. I am not a fan of this either, because it might increase the page size a lot.

So, should we add for background images or not? @hellofromtonya can this be done only for pictures and ignore the background images for now?

[GeekPress]

Let's do the noscript for picture.

The noscript for background url is useless as no SEO impact. The background isn't indexed.

[crystinutzaa]

PR is on Lazyload-common package:
https://github.com/wp-media/rocket-lazyload-common/pull/74