WordPress.org

Make WordPress Core

#35102 closed defect (bug) (fixed)

Responsive images support for external URLs

Reported by: polevaultweb Owned by: joemcgill
Milestone: 4.4.1 Priority: normal
Severity: normal Version: 4.4
Component: Media Keywords: has-patch fixed-major
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Since the introduction of [35821] in #34898, the ability for srcsets to be added to images with external src URLs has been stopped. For example, our plugin https://wordpress.org/plugins/amazon-s3-and-cloudfront/ replaces local URLs with S3 ones in post content, currently srcset will not be added.

Attachments (1)

35102.diff (652 bytes) - added by polevaultweb 16 months ago.

Download all attachments as: .zip

Change History (14)

@polevaultweb
16 months ago

#1 @SergeyBiryukov
16 months ago

  • Description modified (diff)

#2 @SergeyBiryukov
16 months ago

  • Milestone changed from Awaiting Review to 4.4.1

#3 @polevaultweb
16 months ago

  • Keywords has-patch needs-testing added

The patch adds a filter so the check to see if the image can have srcset added to it can be overridden.

Last edited 16 months ago by polevaultweb (previous) (diff)

#4 @joemcgill
16 months ago

  • Owner set to joemcgill
  • Status changed from new to reviewing

Thanks for the report @polevaultweb. Can you give more information about how your plugin replaces the local URLs? If you can point me to some code that I could look at, that would be helpful as well.

WordPress makes use of the attachment ID information found in the class attribute of the image markup to decide which alternate image sizes should be loaded in a srcset attribute. To avoid loading the wrong images when the ID is incorrect (e.g., if the markup was imported from another WP installation), we check to make sure the image src matches one of the image sizes listed in the attachment metadata for that ID. See #34898 and a related bug that might be causing issues for you #35045.

#5 @polevaultweb
16 months ago

No problem @joemcgill! WP Offload S3 (https://wordpress.org/plugins/amazon-s3-and-cloudfront/) copies local files to S3 and then replaces local URLs in post_content via the database, and also hooks into wp_get_attachment_url to swap local for S3 URLs. The code in [35821] generates URLs for images locally based on image sizes so the image src can be searched for, therefore our S3 URLs will never be found and srcset applied.

Let me know if you need anything further.

Last edited 16 months ago by polevaultweb (previous) (diff)

#6 @joemcgill
16 months ago

After a brief Slack chat with @polevaultweb it seems like this use case is something that should be handled by the plugin itself, but after r35821, the wp_calculate_image_srcset filter is no longer reachable because we bail early if the src doesn't match one of the available sizes in the attachment meta.

@polevaultweb I wonder if a workaround would be to filter wp_get_attachment_metadata to insert your offloaded files so wp_image_add_srcset_and_sizes() can find the corresponding image sizes?

#7 @polevaultweb
16 months ago

Actually, the change proposed in #35045 to just compare filenames will work for our use case. This can be closed once #35045 is in, just in case other scenarios aren't covered by it.

Last edited 16 months ago by SergeyBiryukov (previous) (diff)

#8 @joemcgill
16 months ago

  • Status changed from reviewing to accepted

Thanks @polevaultweb. I'm going to keep this ticket open for now and look into ways we might be able to make the filter in wp_calculate_image_srcset reachable for cases like these.

#9 @azaozz
16 months ago

See https://core.trac.wordpress.org/ticket/35045#comment:18

This is quite close to #35045 and the solution for one of them should fix the other.

This ticket was mentioned in Slack in #feature-respimg by joemcgill. View the logs.


16 months ago

#11 @azaozz
16 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 36121:

Responsive images: fix the check whether the attachment meta matches the image src to work with http/https and CDNs.

Props webaware, joemcgill, azaozz.
Fixes #35045 and #35102 for trunk.

#12 @azaozz
16 months ago

  • Keywords fixed-major added; needs-testing removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopen for 4.4.

#13 @dd32
16 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 36152:

Responsive images: fix the check whether the attachment meta matches the image src to work with http/https and CDNs.

Merges [36121] to the 4.4 branch.
Props webaware, joemcgill, azaozz.
Fixes #35045 and #35102.

Note: See TracTickets for help on using tickets.