Photon: Image sizes added based on containing element, not $content_width

laurelfulford commented 7 years ago

An issue has been reported with the Espied theme on self-hosted sites, where Photon is enabled.

On the Portfolio Project pages, the theme uses a larger image size, and uses JavaScript to add a CSS class for images over 1272px in width, which is used to style the images larger than the content. You can see an example of that here:

https://espieddemo.wordpress.com/portfolio/chrome-for-ios7-2/

The special class is being applied to self-hosted sites running Photon, but the image is not displaying at the correct size.

Steps to reproduce the issue

On a self-hosted, non-local site, apply the Espied theme.
Make sure Jetpack is installed. Under Jetpack > Settings, turn off "Speed up images and photos".
Create a Portfolio Project, and insert an image larger than 1272px wide. Make sure to set the image is set to display at the 'Custom' or 'Full Size'.
Publish the project, and view on the front end. You'll see a larger image:

The output for the image will look something like:

Go back to Jetpack > Settings, and turn on "Speed up images and photos".
View the portfolio post again.

What I expected

That the image would look the same.

What happened instead

The image displays at a smaller size. However, the JavaScript in the theme still applies the class to the image, as though it's a larger image, so it's outdented to the left:

The output for this image is:

The problematic bit is the width="552" height="414" at the end.

I've experimented with this a bit. Adjusting the $content_width for the theme does not affect this. However, changing the width of the containing element via CSS does.

For example, if I remove this from the style.css, the image will display at full width:

@media screen and (min-width: 37.5em) {
    .page-content,
    .entry-content,
    .entry-summary {
        width: 552px;
    }
}

It will still include an inline width attribute, but it will use the width set by the theme's $content_width.

If I change the width to 800, the width attribute will match that. If I change it to something higher than the image, it'll stick to the width set by the $content_width.

In Espied, the issue seems to be that Photon gets the image's width on load, and uses that; then the theme's JavaScript checks to see the actual image size (which is still large, even though a width attribute is being used to make it look smaller) and adds the class used by the theme for large images. Then the styles are applied, but the image stays small.

I was wondering why the size is being inserted? It doesn't actually change the image's physical size (which uses the $content_width), just the size it displays at. If it's to make sure it doesn't display too large, themes should be doing that via a max-width in the CSS.

It may be possible to strip out the height and width attributes in the theme via JavaScript, but that seemed a bit hacky. I wanted to double-check why Photon's inserting the sizes first.

laurelfulford commented 7 years ago

Related theme tickets:

4024-wpcom-themes 3965-wpcom-themes

jeherve commented 7 years ago

Related: #4175

laurelfulford commented 6 years ago

There's another report of this, with the Intergalactic 2 theme:

5013-wpcom-themes, 972453-zd

sixhours commented 6 years ago

When this is fixed, this thread needs follow up: https://wordpress.org/support/topic/portfolio-image-size-problem-with-photon-plugin?replies=1

zinigor commented 6 years ago

Related: 973272-zen In this particular case Photon overrides the element width and height attributes and just sets the width to the parent element.

thesacredpath commented 6 years ago

Wondering if this issue is going to be fixed. A self-hosted user first posted about it here, https://wordpress.org/support/topic/full-image-and-jetpack-photon/, and then reposted just recently here, https://wordpress.org/support/topic/conflict-with-photon-still-not-fixed/.

jeherve commented 6 years ago

@thesacredpath We don't have an ETA at the moment, but we will update this issue when we make some progress on this.

cc @RavanH

thesacredpath commented 6 years ago

Thanks @jeherve, I'll let the user know.

RavanH commented 6 years ago

Thanks @jeherve for alerting me to this thread. I'm currently using the CSS (added to Custom CSS) kindly suggested by @dcoleonline on https://wordpress.org/support/topic/full-image-and-jetpack-photon/ and it seems to work well...

stale[bot] commented 6 years ago

This issue has been marked as stale. This happened because:

It has been inactive in the past 6 months.
It hasn’t been labeled `[Pri] Blocker`, `[Pri] High`.

No further action is needed. But it's worth checking if this ticket has clear reproduction steps and it is still reproducible. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.

stale[bot] commented 5 years ago

This issue has been marked as stale. This happened because:

It has been inactive in the past 6 months.
It hasn’t been labeled `[Pri] Blocker`, `[Pri] High`.

No further action is needed. But it's worth checking if this ticket has clear reproduction steps and it is still reproducible. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.

Robertght commented 3 years ago

This is still happening. Found it in #4284341-zen with Morden.

sguzovskaia commented 2 years ago

Another one. Found it in #4645708-zen with Exford.

Automattic / jetpack