Hi,

it seems that images rendered with Picture module are always stretched to 100% width in Chrome. If they aren't limited with CSS (e. g. max-width=100% of parent container) they go to the edge of the screen.

Here is the example of the code:

<img srcset="http://www.example.com/myimage.jpg 640w" alt="My Image" title="My Image">

What I want is just the original file, without any modifications. It displays fine in Firefox, even in IE, but Chrome stretches it far wider than 640px. If I remove these 640w in developer tools, then everything works fine.

You can test an example here:

http://jsfiddle.net/919o5559/

Is it Chrome's bug or something in Picture module that prevents it from rendering images correctly? Is there a way to completely remove width parameter (640w)?

CommentFileSizeAuthor
#5 2449033-5-add-sizes-attribute-correctly.patch562 bytesDeFr

Comments

attiks’s picture

This sounds like a chrome bug, I reported it to the ricg, https://github.com/ResponsiveImagesCG/picture-element/issues/262

attiks’s picture

Got feedback: The default/fallback value for sizes is "100vw", so no bug.

Since chrome is the only browser with native implementation (firefox 38 will have it as well) I guess it might be a polyfill problem

1mundus’s picture

OK, I'll have to wait until they fix it then. If someone needs to get rid of width descriptor, you can remove this from picture.module:

{ $srcset['width'] = $dimensions_clone['width'] . 'w'; }

attiks’s picture

Status: Active » Closed (works as designed)

or enter 640px in sizes

DeFr’s picture

Status: Closed (works as designed) » Needs review
StatusFileSize
new562 bytes

Sorry for re-opening, but I still think there's something wrong on the module side here. The srcset spec is pretty much crystal clear on that point:

If the srcset attribute is present and has any image candidate strings using a width descriptor, the sizes attribute must also be present, and the value must be a valid source size list.

(source: https://html.spec.whatwg.org/multipage/embedded-content.html#the-img-ele... )

So the code is currently not standard compliant, and current Firefox release with experimental picture and srcset attribute enabled through about:config also has the same problem with those images, so it's not really limited to Chrome.

Having to fallback to entering the size manually is a real hassle, given that the code already has everything it needs to be able to output the sizes attribute correctly. Attached patch seems to work for me, and should match the spec: everytime the code add a width descriptor to the image in the srcset, it also adds a matching "sizes" attribute if not present.

attiks’s picture

Version: 7.x-2.9 » 7.x-2.x-dev
Status: Needs review » Needs work

#5We studied the spec some more and it is not really clear, they say sizes is mandatory when using w in srcset, but a bit further they say

f the above algorithm exhausts unparsed sizes list without returning a size value, return 100vw.

SO that's why you're seeing the stretched image.

I'm still not sure what you want to achieve and how you're using the code, is this happening when using a formatter on an image field, or are you calling the theme function directly?

Regarding the patch, what will happen if there are 3 images defined in the srcset, sizes will be something like '100px, 200px, 300px'?

DeFr’s picture

Status: Needs work » Needs review

Right now, it happens when using the default formatter provided by picture.module to an image field. (Actual end usage will be through Scald, which will use picture formatters to provide the media in various resolutions based on the browsing device, but right now I'm trying to get basic things working).

That being said, found the detail that was probably missing from this issue summary: to reproduce the issue, the breakpoint group used in the picture mapping needs to have some devices that are not covered by the media queries (might be argued that this is a configuration error).

What's currently happening in that case is that if all the <source /> elements have media queries that are rejected, then the browser is falling back to the <img /> tag in the picture element ; that tag have an srcset but no sizes, and thus displays incorrectly.

To reproduce, create two breakpoints for mobile and tablet, create a breakpoint group with those two breakpoints, and then a picture mapping based on that group. Then, visit the site with Firefox and Chrome in a desktop wide window ; in both case, the image style associated to the last breakpoint in the group is used, but in Chrome, the image is as wide as the browser ; in stock Firefox or anything using the polyfill, the image will only be as wide as the image style makes it.

Given that the code changed in the patch is in the fallback <img /> case, I think there can't be more than one image in that srcset. There's no foreach loop around that part of the code, and right under it there's a '#srcset' => array($srcsect) , so as far as I can tell, there can't be more than one width.

Edit: Edited to put the various HTML tags in code tags, to avoid avoid a bunch of missing image indicators

attiks’s picture

Now I understand, default behavior is to display the fallback if there's no match, but since we're using srcset inside the fallback it doesn't behave like it should, the fix is easy, use src inside the fallback. The problem is that it will force a double download on browsers without native support.

Did you test your patch in multiple browsers?

<picture>
  <source media="max-width: 100px" srcset="https://www.drupal.org/sites/all/themes/bluecheese/images/world-map.png 450w" />
  <img src="https://www.drupal.org/sites/all/themes/bluecheese/images/sprites.png" alt="My Image" title="My Image">
</picture>
DeFr’s picture

I've tested in stock Firefox 37, Firefox 37 with experimental srcset and figure support enabled and Chrome. I don't have an Internet Explorer on hand right now, but would be able to test laster today if you want.

attiks’s picture

#9 It would be great if you could test in IE as well, just to be sure that nothing will break;

DeFr’s picture

Tested on IE7 through IE11, no behavior changes ; also checked on Safari 8 for good measures, no change there either.

attiks’s picture

I did some more thinking and talked to Bruce yesterday, I think we need to change the fallback mechanism so it uses src instead of srcset and avoiding this problem. I created an issue for Drupal 8, #2481637: Use src in fallback image.

Since there are already a lot of installs we need to make this behavior optional, so I propose that change the fallback the src.

Related note: sizes is now mandatory according to the spec, so we need to change the UI as well.

I'll try to make some time next week to write a patch, unless somebody beats me

jerry’s picture

The patch at #5 is working for me as well, and also applies correctly to the current released version (7.x-2.9).

  • attiks committed 23b06aa on 7.x-2.x
    Fixed #2449033, #2280315: Use src as fallback
    
attiks’s picture

Status: Needs review » Fixed

  • attiks committed 49b2db7 on 7.x-2.x
    Fixed #2449033: Always output sizes
    

  • attiks committed bba85a0 on 7.x-2.x
    Fixed #2449033, #2280315: Use src as fallback
    

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.