Photon: srcset support for post thumbnails #2919

ChrisBegley · 2015-10-29T03:53:39Z

I'm a huge fan of Photon and I've been following the responsive images project for the 4.4 release. I just tested out the latest Photon changes in the Master build of Jetpack with WordPress 4.4-beta2-35426 and the latest version of TwentySixteen. I noticed a few problems, here's my test site:
http://super.techyourworld.com/4-4-responsive-images-test/

It doesn't look like Photon is working on the post thumbnails. With Photon disabled, the scrset and sizes attributes are added as expected:

But with Photon activated, it strips out the srcset and sizes attributes:

<img src="http://i1.wp.com/super.techyourworld.com/wp-content/uploads/2015/10/Batman_1989_0331.jpg?w=1200&quality=85&strip=info" class="attachment-post-thumbnail size-post-thumbnail wp-post-image" alt="Batman_1989_0331" data-id="5459">

This only appears to be happening on post thumbnails. The image I inserted into the actual post displays srcset and sizes, even with Photon activated:

The text was updated successfully, but these errors were encountered:

kraftbj · 2015-10-29T04:42:34Z

Related: #2804 , which we've been tackling to add Photon + srcset support.

Confirmed that we're not playing nicely with Twenty Sixteen's post thumbnail. Not sure why yet :)

ChrisBegley · 2015-10-29T04:44:26Z

Cool. I'm not a developer but I'd be more than happy to help out any way I can by testing.

kraftbj · 2015-10-29T04:49:45Z

Thanks @ChrisBegley! Core thumbnail retrieval happens in filter_image_downsize via the image_downsize filter, which looks like we need to adapt for srcset. This will be a little trickier since our support elsewhere will just be ignored if not running 4.4, while this would need to be versioned.

Confirmed the general area by trying add_filter( 'jetpack_photon_override_image_downsize', '__return_true' );, which short-circuits our filter on image_downsize, which returns the expected tags.

Notes for testing: Test on a single post page, not an archive, unless you also have Carousel deactivated. Of note, when we disable our filter, the srcset values are Photonized, just not the img src tag.

kraftbj · 2015-11-10T17:22:47Z

This is actually a result of #2281. With the image_downsize filter, the URL, width, and height are returned as an array. Typically, all three have values when used generically with WordPress.

When Photon returns it, we return the URL, and, usually, false for both the width and height (see discussion on 2281 for the rationale).

The functions that filter and insert the srcset into the image tag for the thumbnail, though, use those values.

dereksmart · 2015-11-18T22:00:26Z

@ChrisBegley, we've merged a patch for this in our master and master-stable branch. I'd like to take you up on your offer to test it out :)

If you'd rather wait for a beta, we'll be releasing one within a day or two.

Thanks in advance!

ChrisBegley · 2015-11-18T22:16:52Z

No problem! I just tested it out. The srcset attributes are added to the post thumbnails now. However, it's not respecting the crops of the thumbnails. It's adding "?w" instead of "?resize" to the srcset Photon URLs. For example:

This:
http://i1.wp.com/super.techyourworld.com/wp-content/uploads/2015/10/Batman_1989_0331.jpg?w=408

Should be this:
http://i1.wp.com/super.techyourworld.com/wp-content/uploads/2015/10/Batman_1989_0331.jpg?resize=408,227

This is what I have in my functions.php file for the homepage thumbnails:

add_image_size( 'home-panel', 408, 227, true );
add_image_size( 'home-panel-300', 300, 167, true );
add_image_size( 'home-panel-353', 353, 196, true );
add_image_size( 'home-panel-392', 392, 218, true );
add_image_size( 'home-panel-555', 555, 309, true );
add_image_size( 'home-panel-600', 600, 334, true );
add_image_size( 'home-panel-706', 706, 393, true );
add_image_size( 'home-panel-784', 784, 436, true );
add_image_size( 'home-panel-816', 816, 454, true );
add_image_size( 'home-panel-900', 900, 501, true );
add_image_size( 'home-panel-1059', 1059, 589, true );
add_image_size( 'home-panel-1110', 1110, 618, true );
add_image_size( 'home-panel-1176', 1176, 654, true );
add_image_size( 'home-panel-1224', 1224, 681, true );
add_image_size( 'home-panel-1665', 1665, 926, true );

kraftbj · 2015-11-18T22:23:22Z

@ChrisBegley Is that the primary src, the srcset(s), or all of them?

ChrisBegley · 2015-11-18T22:26:20Z

@kraftbj it's only the srcsets that aren't respecting the thumbnail crops. The primary src is correct. Here's what it's outputting:

kraftbj · 2015-11-18T22:34:29Z

Well, that's good news... this specific issue is being resolved, but failing at https://github.com/Automattic/jetpack/blob/master/class.photon.php#L582 that was fixed previously :)

Not sure if I'll be able to hit this today, but noting for @dereksmart and @zinigor that we'd need to checkout what is available to us via the core filter, probably, and set the args smarter than just using the w.

(Noting that the other issue should probably be re-opened, but I'm okay tracking this here since we'll get it fixed in the next day or so anyhow. #2804 )

dereksmart · 2015-11-18T22:56:54Z

There was some discussion in #2885 about whether we should be grabbing the height as well through the 2nd param of the filter $size_array, and went against it b/c it "worked" with minimal intrusion. seems like we need them for custom image sizes. Let's try and add them in.

dereksmart · 2015-11-18T22:57:15Z

of course as I was writing this @kraftbj wrote a patch <3

dereksmart · 2015-11-19T16:59:05Z

@ChrisBegley we've pushed another patch for height dimensions. Mind giving it another spin?

ChrisBegley · 2015-11-19T21:18:35Z

@dereksmart everything looks good!

One side note... When I just overwrote all of the files in the plugin folder, it was acting like Photon wasn't active. I had to deactivate Photon and reactivate Photon in order to see the changes here, but everything looks like it's working fine now.

kraftbj added [Feature] Photon aka "Image CDN". Feature developed in the Image CDN package and shipped in multiple plugins [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Oct 29, 2015

kraftbj changed the title ~~Latest Photon changes + 4.4 responsive images~~ Photon: srcset support for post thumbnails Oct 29, 2015

kraftbj added this to the 3.8.1 milestone Oct 29, 2015

zinigor self-assigned this Nov 6, 2015

zinigor mentioned this issue Nov 17, 2015

Photon: made the downsize filter return resulting image dimensions. #3029

Merged

zinigor added the [Status] In Progress label Nov 17, 2015

dereksmart closed this as completed in #3029 Nov 18, 2015

dereksmart removed the [Status] In Progress label Nov 18, 2015

kraftbj mentioned this issue Nov 18, 2015

Photon does not return dimensions with wp_get_attachment_image_src #2281

Closed

kraftbj reopened this Nov 18, 2015

kraftbj mentioned this issue Nov 18, 2015

Photon: srcset to use custom thumbnail sizes #3040

Merged

kraftbj assigned kraftbj and unassigned zinigor Nov 18, 2015

kraftbj added the [Status] In Progress label Nov 18, 2015

kraftbj assigned zinigor and unassigned kraftbj Nov 18, 2015

joemcgill mentioned this issue Nov 19, 2015

Add custom srcset + sizes values to post thumbnails? ResponsiveImagesCG/wp-tevko-responsive-images#255

Open

dereksmart closed this as completed in #3040 Nov 19, 2015

dereksmart removed the [Status] In Progress label Nov 19, 2015

zinigor mentioned this issue Dec 1, 2015

Hook Photon into wp_get_attachment_url() #946

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Photon: srcset support for post thumbnails #2919

Photon: srcset support for post thumbnails #2919

ChrisBegley commented Oct 29, 2015

kraftbj commented Oct 29, 2015

ChrisBegley commented Oct 29, 2015

kraftbj commented Oct 29, 2015

kraftbj commented Nov 10, 2015

dereksmart commented Nov 18, 2015

ChrisBegley commented Nov 18, 2015

kraftbj commented Nov 18, 2015

ChrisBegley commented Nov 18, 2015

kraftbj commented Nov 18, 2015

dereksmart commented Nov 18, 2015

dereksmart commented Nov 18, 2015

dereksmart commented Nov 19, 2015

ChrisBegley commented Nov 19, 2015

Photon: srcset support for post thumbnails #2919

Photon: srcset support for post thumbnails #2919

Comments

ChrisBegley commented Oct 29, 2015

kraftbj commented Oct 29, 2015

ChrisBegley commented Oct 29, 2015

kraftbj commented Oct 29, 2015

kraftbj commented Nov 10, 2015

dereksmart commented Nov 18, 2015

ChrisBegley commented Nov 18, 2015

kraftbj commented Nov 18, 2015

ChrisBegley commented Nov 18, 2015

kraftbj commented Nov 18, 2015

dereksmart commented Nov 18, 2015

dereksmart commented Nov 18, 2015

dereksmart commented Nov 19, 2015

ChrisBegley commented Nov 19, 2015