[wp-trac] [WordPress Trac] #35030: Responsive Images: wrong source selected in iOS8
WordPress Trac
noreply at wordpress.org
Fri Mar 18 18:56:54 UTC 2016
#35030: Responsive Images: wrong source selected in iOS8
--------------------------------------+------------------------
Reporter: tonysandwich | Owner: joemcgill
Type: defect (bug) | Status: reopened
Priority: normal | Milestone: 4.5
Component: Media | Version: 4.4
Severity: normal | Resolution:
Keywords: has-patch has-unit-tests | Focuses:
--------------------------------------+------------------------
Comment (by azaozz):
Yep, the changes in 35030.4.patch look good, and array() + array() is
better there than array_merge().
However the changes to the tests need more.
1. Not sure what is being tested with
`test_wp_calculate_image_srcset_src_first()`. It is not needed to
specifically test if the image src is first in srcset. That is being
tested by all other srcset tests already.
Another big problem there is that some of the test data is randomized.
This means it may introduce intermittent failures, i.e. some times the
test will pass, other times it will fail. That's really bad for testing :)
If there are 10 cases, all need to be tested every time so we can see
which one fails.
2. There is a lot of repetition in most of the srcset testing functions
when generating comparison strings. It would be good to "DRY" that and
include the functionality from the new `_src_first()` helper. This is
pretty much the same in 4 places:
{{{
$year_month = date('Y/m');
$uploads_dir = 'http://' . WP_TESTS_DOMAIN . '/wp-content/uploads/';
// Set up test cases for all expected size names.
$intermediates = array( 'medium', 'medium_large', 'large', 'full' );
foreach ( $_wp_additional_image_sizes as $name => $additional_size ) {
if ( ! $_wp_additional_image_sizes[$name]['crop'] || 0 ===
$_wp_additional_image_sizes[$name]['height'] ) {
$intermediates[] = $name;
}
}
$expected = "";
foreach ( $image_meta['sizes'] as $name => $size ) {
// Whitelist the sizes that should be included so we pick up
'medium_large' in 4.4.
if ( in_array( $name, $intermediates ) ) {
$expected .= $uploads_dir . $year_month . '/' .
$size['file'] . ' ' . $size['width'] . 'w, ';
}
}
$expected .= $uploads_dir . $image_meta['file'] . ' ' .
$image_meta['width'] .'w';
}}}
Of course, that is a test change so it can be added in RC too.
--
Ticket URL: <https://core.trac.wordpress.org/ticket/35030#comment:31>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list