Provide option to use srcset and/or sizes attributes on img tag instead of the picture element

I totally agree. The responsive image module enables you to provide the srcset attribute: #2262863: Add srcset to template_preprocess_image But it does not provide an option to use srcset instead of the picture element in it's formatter.

I am moving this issue to Drupal core and changing the title accordingly.

You're right, but it depends on how well we want to support old browsers, the problem with using srcset on img is that the src will get prefetched by most browsers. We need to decide how we want to handle this.

See also #2343351: Make picture polyfill optional

You're right.

The idea is to switch the output during rendering, once #2260061: Responsive image module does not support sizes/picture polyfill 2.2 is committed we can start working on this

#2260061: Responsive image module does not support sizes/picture polyfill 2.2 got committed (YAY!), hence this is now unblocked :)

My initial idea was to define a new twig template, but I have no idea on how to switch twig templates inside template_preprocess_responsive_image.

Or is it easier/better to handle this inside the twig template?

Assigning to Fabian Franz, to get his thought on how to handle this elegantly in Twig.

But shouldn't this depend on the Responsive Image Style being used? We don't wan to (and can't?) make just *any* Responsive Image Style use this, right? I wonder if we can detect whether art direction is being used, and then use picture?

This will only be done when all of the following are true:

1) If there would be only one source element AND
2) There is no need for an image type attribute AND
3) The mapping type is sizes, THEN

So it will indirectly depend on the responsive image style, since the art direction case will use multiple mappings, it will always output using the picture tag.

Alright. Thanks, that's excellent info :)

Then I suspect the best way to do this is something like having the responsive image preprocess function modify the template file to be used. Not sure if that's possible though. Hoping for Fabian's feedback.

Given the business rules, one tricky issue is rule 2: how do we determine if there is no need for an image type attribute? My memory is that an image type attribute is always output, even if all the images are jpg. In theory you could determine a certain set of image types that would meet the qualifications for rule 2, but that could get tricky.

I think you could theoretically handle the markup language in a single twig template: it's determining what markup to display, so that is probably appropriate for logic within a template. That might be more straightforward than switching which template is used.

Indeed. 2. is the hardest. Currently the only time the type attribute is required (when there is only 1 <source> tag) is when we're dealing with WebP images. But it feels wrong to hardcode that. In all other cases I think we should be fine.

GD (and by extension Drupal) doesn't really support WebP yet. But there is an issue to add WebP support to Drupal 8 regardless of that (#2340699: Let GDToolkit support WEBP image format, #1443070: Add support for popular e-book formats, Google web formats, mkv and mka in file_default_mimetype_mapping()). So I'm not sure how we should deal with the "There is no need for an image type attribute" condition. Any ideas/suggestions?

As for switching the twig template (vs all logic in one template): it would be possible via hook_theme_suggestions_HOOK. The problem is that this hook is called before template_preprocess_responsive_image. This means we don't yet know what our output will be (one <source> or many, type attribute or not, ...). We could do the preprocessing here as well to find out, but that would become an expensive operation. Best way to go then would be setting a flag in template_preprocess_responsive_image and put the rest of the logic in responsive-image.html.twig I'm afraid...

#12 I think we can ignore the mime type if there's only one in play, so if somebody chooses to only use webP, we output an img tag.

The image type attribute is not mandatory, but in the case of webP the output will probably be broken on some browsers.

#13 I think the easiest is to add a switch ($output_image_tag) and add a condition in twig template.

Discussed the conditions when to switch with @attiks. They should be:

1. There is only one <source> tag.
2. The media attribute is empty.

The type attribute doesn't really matter because there is only one source, which means the type attribute won't have any influence (as described in #14)

I started working on the patch.

This patch should work in theory but there are no tests yet so NW.

Honestly, I think that given the difficulty of implementing these business rules, this is a dangerous road to go down.

One really good usage of the picture module, for example, is to provide an svg in a source element with a type attribute, and then having a fallback with the img element for the browsers that don't understand svg. The same could be true for webp. So simply switching to an img element due to there being only one source is going to miss some useful use cases.

Having a single img element vs a picture element with one source is a nice to have, not a must have. I've confirmed this by talking to those working on implementing this in browsers. The two are functionally the same. So while a standalone tag with only an img is nice, in that there's less markup, there isn't a huge benefit to doing that. So I'm leery of automatic switching to an img element when that could cause some harm.

#17 Valid points, but - alas - you cannot upload svg files as images into Drupal, same goes for WebP, so this is a theoretical problem.

That being said, it might be better no to rush this patch, unless we're sure where to go.

PS: You're still supporting browser without svg? ;-)

#17: Isn't another reason the fact that <img srcset> has broader browser support already than <picture>? I think that's an important factor to take into account to: it should mean better front-end performance for those sites?

#18:

you cannot upload svg files as images into Drupal, same goes for WebP, so this is a theoretical problem

Isn't this statement only true for Drupal 7 & 8's default list of allowed file types to be uploaded? I.e. anybody can change that list and allow SVGs to be uploaded.

We have quite a while before Drupal 8 comes out and the marginal difference in browser support at that point (which may not even exist) is not a great reason to neuter image type switching.

Allowing a front end developer to choose to use image vs picture? Sure. Forcing that choice? Seems semi-dodgy.

Ok, just wanted to make sure that factor was taken into account!

#19 file types are hard coded at least in Drupal 7, to be honest didn't check D8, but I doubt if uploading svg and webp will work out of the box.

#22: fresh D8 install, go to /admin/structure/types/manage/article/fields/node.article.field_image, see the Allowed file extensions setting.

Analogous in D7.

#23 and did you try to output a webp or svg, preferably using an image style ;-)

#24: AHA! Excellent point! :D Stupid of me not to see that. Thanks for the insight!

I think this is a very important issue:
For the majority of the images on the web, the picture tag is the wrong solution.
The picture element must be used only to get an alternative image for different resolutions and not a different size.

The bad use of the picture element is harmful and semantically incorrect.

http://blog.cloudfour.com/dont-use-picture-most-of-the-time/

To summarize:

• The picture specification contains more than just the
  element. Think of it as the responsive images specification.
• For most responsive images, you shouldn’t use the
  element. You should use srcset and/or sizes.
• The way to know when to use the
  element is based on the use case you’re trying to solve.
• If you’re solving for the art direction use case, use the
  element. Anything else, you should stick to srcset and sizes.
• Getting this right early—before we have thousands of pages using
  incorrectly—is critical for the future of the web.

Others source:
http://www.smashingmagazine.com/2014/05/14/responsive-images-done-right-...
https://html.spec.whatwg.org/multipage/embedded-content.html

#26 You're absolutely right, but see my comment in #2, if we do this browsers without picture support will download 2 images.

So before I start working on this, we need to decide if:
- we still want a polyfill
- we don't mind people downloading 2 images

I understand.

But is not possible not use polyfill by default (many browser are implementing srcset and picture) and add the option to choose polyfill as fallback?

#28 I think it has been discussed in #2343351: Make picture polyfill optional to add a switch to toggle the polyfill inclusion. I guess work on this can be started.

This might be a good solution.

The issue in my mind is not whether or not using img + srcset/sizes is good. It definitely is. I think #26 overstates the case: using picture isn't always necessary, but ultimately it works pretty much the same.

The bigger problem is deciding when to output img vs picture. I don't think we can automatically decide this: there has to be user choice to use the img element. If we can provide an option for that, probably in the picture mapping UI, or in a theme function, great. But I don't think making that decision automatically is going to be easy or always correct.

I'm also not clear why if we used img that would mean a double download. Couldn't we just srcset + sizes with no src plus the polyfill? I'm not clear on how that's going to result in a double download. Maybe I'm missing something.

Browser support is growing, but I don't think we can pull the polyfill right now. I think that's a call we'll likely have to make pretty late in the game.

The issue in my mind is not whether or not using img + srcset/sizes is good. It definitely is. I think #26 overstates the case: using picture isn't always necessary, but ultimately it works pretty much the same.

Picture tag works but is not semantically correct.
Also p tag can be displayed like h1 tag (with certain css rules) but is not semantically correct.
http://lists.w3.org/Archives/Public/public-respimg/2014Sep/0015.html

The bigger problem is deciding when to output img vs picture. I don't think we can automatically decide this: there has to be user choice to use the img element. If we can provide an option for that, probably in the picture mapping UI, or in a theme function, great. But I don't think making that decision automatically is going to be easy or always correct.

In my opinion the default choose must be img + srcset/sizes. The tag picture was created for specific uses (i.e.: for the art direction use case), while the img (+ srcset/sizes) tag to be uses for general-purpose of responsive images (i.e.: same image but with different size).
However the best solution can be to add an option in Manage display.

I'm also not clear why if we used img that would mean a double download. Couldn't we just srcset + sizes with no src plus the polyfill? I'm not clear on how that's going to result in a double download. Maybe I'm missing something.

Browser support is growing, but I don't think we can pull the polyfill right now. I think that's a call we'll likely have to make pretty late in the game.

Time is running out:
http://googlewebmastercentral.blogspot.it/2015/02/finding-more-mobile-fr...
To test some sites: https://developers.google.com/speed/pagespeed/insights/
For SEO this is extremely important.

#31

I'm also not clear why if we used img that would mean a double download. Couldn't we just srcset + sizes with no src plus the polyfill? I'm not clear on how that's going to result in a double download. Maybe I'm missing something.

Outputting no source will avoid the double download, but isn't correct.

Questions (and mine POV) at hand are:

1. Do we want to output using img tag? Yes
2. Do we use a separate formatter to do it? Maybe
3. Do we use a setting on the mapping to do it? Don't think it is the right place to do it
4. Do we use a global setting that will output img tag if possible? Maybe
5. Do we output img tag if possible? I prefer this one
6. Do we leave img src blank or not? I guess so, or make it controllable using a global option

+1 attiks

What's the conclusion? I.e. what do we want to do? At the very least: what's the first small step that should happen, that is agreed upon by all?

Not a fan of a global setting to switch between picture and img output. Really think this needs to be a per-mapping decision.

Art direction is a critical use case for picture, yes, but it's not the only one. Providing fallbacks for varying image types is another important use case.

Picture vs img is in no way similar to h1 vs p. Those elements have important accessibility differences between them: that is not at all true for picture vs img. I very much disagree that using picture, even for viewport-based switching, is semantically incorrect. Providing one source with sizes and srcset with a default on the controlling img is an okay thing to do. Is an img element by itself more efficient? Sure. But that doesn't make picture usage incorrect.

Nor is using picture something that is going to cause an SEO problem. I don't see anything in those links that indicates that. Picture is a good tool for responsive design (and so is img + sizes/srcset.

Time is running out. Right now we have zero UI for using sizes + srcset. Solving that is really important. If we can provide an option for outputting via img rather than picture, then great. Whether that's in the mapping or is an option on the formatter and in the theme function I'm agnostic about.

Anyhow, so as to what we can agree on.

I'm fine with providing an option for img output. I think that should be done on a per mapping basis, not globally. If we can do that on top of getting the rest of our tasks done, then great.

There are lots of ways to do things. I think it would be best to keep the focus on "let's provide another good option for markup" rather than "how we're doing this is wrong and must be changed or the world ends." We all want the same thing, great tools for using responsive images. Let's focus on that. :)

img/srcset and a fully featured picture element have two distinct uses IMO, the line being control. srcset is supposed to provide control to the browser to make an informed decision on which source to use, optimizing for size/resolution/performance. picture gives control to the developer in cases like art direction or mime type. Currently it seems the D8 version only allows for one image source per source element, addressing the latter use case. Apart from wanting the most semantic and correct markup, not empowering the browser to make educated decisions puts more work on the side of the developer and wouldn't leverage new features browsers may factor into their decisions. This could mean determining source based on bandwidth or taking into account user settings based on performance.

I like the solution of having two separate field formatters, as is happening in the D7 version. We are talking about two different sets of markup, though there is some overlap. While the D7 version doesn't yet support img/srcset (https://www.drupal.org/node/2450863) I like the mental distinction two field formatters creates. If you need to map sources or source sets to breakpoints (or mime types), you need a responsive image style and use the current field formatter. If all you want to do is inform the browser, you use the field formatter that takes care of that.

As for whether a setting should be global or per mapping, my vote is global. Since the decision right now is basically "do I care about the markup" or "do I care about performance", I can't think of a use case where that decision changes per image. I would be interested in the argument against that though.

So to answer #35, what I would like is global settings addressing the duplicate download issue. Adding a second field formatter similar to "Images w/ sizes" as is in D7 (though I'd push for a name change). This, I believe, would require another template for the new markup. After that we could extend this functionality to the responsive image styles interface as to allow multiple sources per source tag, which I don't believe is currently possible.

Are there any issues with this approach that would prevent a patch being considered?

I think introducing another image formatter at this point is unlikely to get in for 8.0.x. Just not enough time to make that happen.

This has been swirling around in the back of my mind, though. I'll look through this again to make sure, but my thought is that we may be able to test to see if it makes sense to use just the img element, add the variables that would help make that decision in the preprocess layer, then add some logic in the template to choose between the two. That gives full markup control to themers and makes it easy to change globally or on a case by case basis.

A field formatter with different templates also isn't ideal because a formatter isn't the only way to output responsive images. Sometimes it's necessary to create a responsive image render array in code, as a formatter can't be selected.

If there's an issue getting another formatter in, then I think your proposed solution makes sense. It does feel like that one template will be trying to handle a lot of different use cases, which I don't think is ideal but is perhaps unavoidable.

I'm not sure I follow your last comment. My thought would be that the field formatter would leverage a separate theme hook. That way you can still create an img/srcset render array in code without relying on the field formatter.

If there's a separate theme hook, then yes, that reduces my concern of using a different field formatter.

Just for clarity, that still doesn't mean it's likely to get in 8.0.x correct?

So what I'm thinking is that within template_preprocess_responsive_image, we do the following:

1) Is there more than one array item in $variables['sources']? If so, set $variables['multiple'] = TRUE.

2) If multiple is FALSE, check if the mime types for the derivatives for the single source match the mime type for the fallback style. If so, set $variables['type_difference'] = TRUE.

3) If multiple is FALSE, check if there is a non-empty value for the media query for the single source. If so, set $variables['media_query'] = TRUE.

4) If multiple, type_difference and media_query are all FALSE, check if the mapping type for the single source is sizes. If so, set $variables['srcset_type'] = 'width'.

5) If multiple, type_difference and media_query are all FALSE, check if there is more than one multiplier for the single source. If so, set $variables['srcset_type'] = 'density'.

6) If multiple, type_difference and media_query are all FALSE, and srcset_type is 'width', create a $variables['img_srcset_width_element'] with #theme = 'image' and #srcset with the srcset value for the single source and #sizes with the sizes value for the single source.

7) If multiple, type_difference and media_query are all FALSE, and srcset_type is 'density', create a $variables['img_srcset_density_element'] with #theme = 'image' and #srcset with the srcset value for the single source.

8) Within the Twig template, these values can then be used to choose whether to output a picture with a nested img_element or just a img_srcset_density_element or img_srcset_width_element.

Alternatively, all three options could use img_element, although that then requires a themer to dig into the preprocess to change things, which is not the direction we've been heading with d8 templates.

This does mean there would be no src element on the standalone img elements. As attiks has noted, that's not technically correct, but that's what we're already doing for the controlling img within the picture element, and that's not correct either. Leaving off the src element is what's recommended in the Picturefill guidelines. http://scottjehl.github.io/picturefill/#gotchas I don't see us removing Picturefill anytime soon, definitely not before 8.0.0.

While the above logic is complicated, it does allow us to simplify the markup down to a single img element with no picture or source elements only when necessary. If there are multiple sources, a media query or a discrepancy between the fallback style and the styles on a single source (an indicator that responsive images is being used to create a fallback for a newer image type), we use picture. Otherwise, a nice and simple img with either a srcset with x values or a srcset with w values + sizes.

And if anybody doesn't want to do it that way, they can modify the preprocess or Twig template to get different results. If they want to put a src attribute in there, they can modify the preprocess to do so. Because you really, really should know what you're doing if you want to make that choice, I think it's fine if not better to leave those controls in preprocess/templates rather than creating a UI setting, which is more likely to result in somebody choosing the wrong thing and getting a double download.

I'm guessing we may need a couple helper functions to make this work, but I think it's doable to get this in for 8.0.0 if people this makes sense, and we work fast. In fact we can only get this in for 8.0.0, as template markup is frozen after RC1. So we'd have at best 10 days or so to get this working. I think that might be possible.

It does sound complicated and makes we wonder what the twig will look like, any idea how this will look in twig?

NR for patch in #16, so we can build on that.

Reroll

+++ b/core/modules/responsive_image/responsive_image.module
@@ -178,18 +178,38 @@ function template_preprocess_responsive_image(&$variables) {
+  if (count($variables['sources']) === 1 && !isset($variables['sources'][0]['media'])) {

make sure we have sources

Test is probably failing because there's a test only needing 1 source ;-)

New & improved patch. Added test.

nitpick

+++ b/core/modules/responsive_image/templates/responsive-image.html.twig
@@ -6,6 +6,8 @@
+ * - output_image_tag: Whether or not to output an <img> tag in stead of a

instead

I think it would be good once we've determined there is only source with no media query to also compare the image type(s) for that source versus the image type for the fallback style. Doing that would allow somebody to provide a newer image type, while still providing a fallback to an older image type.

If that proves too difficult, it's not the end of the world. You could still do an image type fallback by providing two different types of image files with a breakpoint using sizes. That should turn into a picture element with two different source element if I understand things right.

I think it would be good once we've determined there is only source with no media query to also compare the image type(s) for that source versus the image type for the fallback style. Doing that would allow somebody to provide a newer image type, while still providing a fallback to an older image type.

I'm not sure I understand. What do we do if the types match and what do we do if they don't match?

I briefly discussed this with @attiks yesterday, and we both weren't sure what you meant. There is no reason why an img tag with a srcset attribute and fallback image where the image types are different would need any special handling in code. This should all just work out of the box if you can create an image style that converts the image type. As far as we know the spec allows it and the browser will decide which image it will pick.

New patch with the nitpick from #53 fixed.

Works great and simplifies the output

+++ b/core/modules/responsive_image/responsive_image.module
@@ -178,18 +178,38 @@ function template_preprocess_responsive_image(&$variables) {
+    // we can output an image tag with the srcset attribute in stead of a

should be "instead of"

Tested this out, this looks good. I think the typo can be fixed on commit or with a quick patch.

Fine with how things are working with image types. Like I said, if you want to provide a fallback from one image type to another, you can still do so: two source elements will be used to make that happen. That's fine.

Comment says tests failed, but on my screen it says everything passed. Resetting to NR to see what happens.

All green

Looks like a testbot hiccup to me. Irrelevant tests fail. RTBC assuming testbot will be green again.

Another hiccup as far as I can tell... Testbot should stop drinking ;-)

This seems like a very good thing to try and get in before RC.

Committed and pushed to 8.0.x. Thanks!

In reviewing this I missed that this change will result in double downloads of image files for responsive images using only the img element with older browsers.

Filed #2580695: Prevent responsive image without picture element from causing double downloads of images as a follow-up.