Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#34898 closed defect (bug) (fixed)

Responsive images broken on edited/imported content

Reported by: kovshenin Owned by: kirasong
Priority: highest omg bbq Milestone: 4.4
Component: Media Version: 4.4
Severity: blocker Keywords: has-patch
Cc: Focuses:

Description

When we're rendering a srcset for responsive images we're relying on the wp-image-* class name, but the class name may not be consistent with attachment IDs when working with imported content or when editing the src attribute manually. This can result in different images shown on different screen sizes or screen densities.

Attachments (3)

34898.diff​ (1.0 KB ) - added by kovshenin 11 years ago.
34898-test.diff​ (1.1 KB ) - added by joemcgill 11 years ago.
Adds unit tests
34898.2.diff​ (2.3 KB ) - added by kirasong 11 years ago.

Download all attachments as: .zip

Change History (16)

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


11 years ago

@kovshenin
11 years ago

#2 @kovshenin
11 years ago

  • Keywords has-patch needs-unit-tests added

In 34898.diff​: Bail if the original src is not in any of the image sizes.

This ticket was mentioned in ​Slack in #core by wonderboymusic. ​View the logs.


11 years ago

#4 @azaozz
11 years ago

Looks good, +1

#5 @joemcgill
11 years ago

  • Keywords has-patch removed

We might be able to streamline things by moving this logic to wp_calculate_image_srcset(), but the patch works and doesn't add any measurable performance overhead. We should add a unit test for this as well.

#6 @kirasong
11 years ago

As you'd expect, it's slightly slower, but within margin of error for profiling.

+1

#7 @kirasong
11 years ago

  • Keywords has-patch added

@joemcgill
11 years ago

Adds unit tests

#8 @joemcgill
11 years ago

  • Keywords needs-unit-tests removed

Add a unit test in 34898-test.diff.

#9 @kirasong
11 years ago

  • Owner set to mikeschroder
  • Status new → assigned

@kirasong
11 years ago

#10 @wonderboymusic
11 years ago

+1 on 34898.2.diff

#11 @kirasong
11 years ago

  • Resolution → fixed
  • Status assigned → closed

In 35820:

Media: Don't generate responsive image attributes if src does not match ID in wp-image- class.

We rely on the wp-image- class to quickly find an attachment ID to add responsive image attributes.
To avoid incorrect images being displayed, do not add these attributes if the src does not match the
meta from the attachment ID in the class.

Props azaozz, kovshenin, joemcgill.
Fixes: #34898.

#12 @ocean90
11 years ago

In 35821:

Media: Don't generate responsive image attributes if src does not match ID in wp-image- class.

We rely on the wp-image- class to quickly find an attachment ID to add responsive image attributes.
To avoid incorrect images being displayed, do not add these attributes if the src does not match the
meta from the attachment ID in the class.

Merge of [35820] to the 4.4 branch.

Props azaozz, kovshenin, joemcgill, mikeschroder.
See #34898.

#13 @johnbillion
11 years ago

  • Version trunk → 4.4
Note: See TracTickets for help on using tickets.