Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

[3.4.0] Width and height attributes shouldn't be set if the sizes attribute is specified #7500

Closed
ausi opened this issue Dec 5, 2014 · 14 comments
Labels
Milestone

Comments

@ausi
Copy link
Member

ausi commented Dec 5, 2014

We should change the if-condition for the width and height attributes from !$this->sources to !$this->sources && empty($this->img['sizes']).

See also https://community.contao.org/de/showthread.php?54305

@leofeyer leofeyer added the defect label Dec 5, 2014
@leofeyer leofeyer added this to the 3.4.1 milestone Dec 5, 2014
@Zeromax
Copy link

Zeromax commented Dec 5, 2014

I disagree with removing it. Always SEO experts tell me that the height and width Attribute is important for search engines like google which Need this for the Picture search. Or am I wrong?

Maybe the view has Change. 😃

@ausi
Copy link
Member Author

ausi commented Dec 6, 2014

Always SEO experts tell me that the height and width Attribute is important for search engines like google which Need this for the Picture search. Or am I wrong?

I think your SEO experts are wrong. The attributes width and height can be important for the performance of the page, but this is only true if the dimensions of the image match up with the attributes and don’t get changed via CSS. Contao already removes the width/height attributes if the <picture> element is used and it should also remove them if the attribute sizes is used, because then the width/height attributes don’t always match the dimensions of the used image.

@fritzmg
Copy link
Contributor

fritzmg commented Dec 7, 2014

but this is only true if the dimensions of the image match up with the attributes and don’t get changed via CSS

This is not true. Even if you change the size of the image via CSS, the image's original dimensions can still be important for the browser, especially if the image is resized via CSS relative to its parent for example. With the width and height attribute set, the browser can let the image's element occupy the correct space within the document, even before the browser knows its size yet by loading the actual resource. However, this behavior is not consistent accross all browsers.

@ausi
Copy link
Member Author

ausi commented Dec 7, 2014

I tested this in many browsers and every one ignored the width and height attributes if the image gets resized via CSS.


Test case, online at madeyourday.net/img-size-test

<!-- index.php -->
<style>
    img {
        width: 10%;
        height: auto;
        border: 1px solid red;
    }
</style>
<img src="image.php?t=<?php echo microtime(true) ?>" width="1028" height="430">
<?php // image.php
sleep(2);
header('Content-Type: image/jpeg');
readfile('image.jpg');

  • Webkit based browsers (Chrome, Opera, Safari, iOS Safari, Android) and IE12 preview render the image at 0 height until it’s loaded.
  • Firefox and IE 6-11 render the image as a square (same height as width) until it’s loaded.

@fritzmg
Copy link
Contributor

fritzmg commented Dec 8, 2014

Yeah, you're right. Would seem like a good feature though, if browsers did this consistently.

@Zeromax
Copy link

Zeromax commented Dec 8, 2014

@ausi Thanks for that explanation. So IMO I don't see any reasons why width and height should be there. So your suggestion is fine for me.

@Aybee
Copy link
Contributor

Aybee commented Dec 8, 2014

What about catching the real size of an img with JS.

imgWidth = myImage.get('width');

Will this still be possible?

@ausi
Copy link
Member Author

ausi commented Dec 8, 2014

If you refer to the .get method of MooTools which is similar to the native .getAttribute you will get null if the attributes are not specified. But you can access the displayed width via .width and the natural width via .naturalWidth once the image is loaded.

@Aybee
Copy link
Contributor

Aybee commented Dec 8, 2014

Ok thank you, I dindn't know about the naturalWidth property. But I have to wait until the image is loaded - correct?

To me it's very new and curious to have no original width and height as attributes anymore. Somehow I don't like it.

@ausi
Copy link
Member Author

ausi commented Dec 8, 2014

But I have to wait until the image is loaded - correct?

Correct.

To me it's very new and curious to have no original width and height as attributes anymore. Somehow I don't like it.

Just don’t use responsive images, then the width and height attributes will still be there.

@aFarkas
Copy link

aFarkas commented Jan 1, 2015

Removing the width/height attributes is the right thing to do.

This is maybe a little bit off topic, but you could add a new feature for developers: add instrinsic ratio HTML/CSS
The reason why self-proclaimed SEO experts are demanding to specify width and height attributes, is because they are running page speed insights on a webpage and using these results to consult with.

In case of adaptive images this page speed insight test doesn't make any sense, but the ratio behind this test can accomplished (minimize reflows by reserving/occupying the width and height space for the image) with the so called CSS intrinsic ratio padding-bottom pattern. Unfortunately this means you need to know the exact image dimension ratio, which isn't often known by the frontend. Therefore it would be huge, if developers could opt-in to let contao generate this markup automatically:

<style>
.contao-intrinsic-box {
    display: block;
    position: relative;
    height: 0;
    overflow: hidden;
}
.contao-intrinsic-box img,
.contao-intrinsic-box video,
.contao-intrinsic-box iframe,
.contao-intrinsic-box object {
    display: block;
    position: absolute;
    top: 0;
    left: 0;
    width: 100%;
    height: 100%;
}
</style>


<span class="contao-intrinsic-box contao-landscape-box" style="padding-bottom: 42.86%">
    <img
        src="http://placehold.it/175x75"
        sizes="foobar"
        srcset="http://placehold.it/175x75 175w,
        http://placehold.it/350x150 350w,
        http://placehold.it/700x300 700w,
        http://placehold.it/1400x600 1400w" />
</span>

This would be really huge. There are also a lot of scripts out there (mostly carousels/sliders, but also isotope/masonry) trying to wait until all images are loaded to get the right dimension of their items. These scripts currently epically fail as soon as it comes to responsive images. With the intrinsic ratio pattern those problems are also automatically solved. What do you think? Should I open a new issue for this feature?

Additionally you can also think of adding the height descriptor. While this descriptor isn't in use by any browser yet, there is some discussion to use this to also solve this problem regardless of the full height descriptor implementation:

<img
        src="http://placehold.it/175x75"
        sizes="foobar"
        srcset="http://placehold.it/175x75 175w 75h,
        http://placehold.it/350x150 150h 350w,
        http://placehold.it/700x300 300h 700w,
        http://placehold.it/1400x600 600h 1400w" />

@ausi
Copy link
Member Author

ausi commented Jan 7, 2015

@aFarkas thanks for your input.

I think this would be a great feature but it’s also a bit complicated. I’m not sure if this should be in the core or in an external extension.

Do you have more information about the height descriptor? Is it backwards compatible with current browser implementations?

Should I open a new issue for this feature?

Yes please, because your idea is a feature and this issue is about a bug which will be fixed in the next patch release.

@leofeyer
Copy link
Member

Fixed in f9995c9.

@ausi Can you please confirm that my patch works? Instead of using

if (!$this->sources && empty($this->img['sizes'])):

I have combined it with the previous if statement:

<?php if (!empty($this->img['sizes'])): ?> sizes="<?php echo $this->img['sizes']; ?>"<?php elseif (!$this->sources): ?> width="<?php echo $this->img['width']; ?>" height="<?php echo $this->img['height']; ?>"<?php endif; ?>

@ausi
Copy link
Member Author

ausi commented Jan 16, 2015

@leofeyer confirmed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants