Opened 8 years ago
Closed 8 years ago
#40010 closed defect (bug) (fixed)
Customize: Template for site icon control fails to check if full image size exists before using
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 4.7.4 | Priority: | normal |
| Severity: | normal | Version: | 4.5 |
| Component: | Customize | Keywords: | has-patch fixed-major |
| Focuses: | Cc: |
Description
Looks like #36749 has shown itself again in the latest core commits.
I've attached a patch, which fixes this issue.
Seems that the template is not checking if data.attachment.sizes.full exists before printing out data.attachment.sizes.full.url. This causes a console error (trying to get property url from a non-object).
To reproduce:
- Open Customizer
- Under site identity upload an image
- Publish
- Refresh the page
Expected:
Page loads normally with new icon
Actual:
Page partially loads, customizer is broken
Attachments (2)
Change History (23)
#2
@
8 years ago
@aussieguy123 This is only happening in a context where Jetpack's Photon service is being used, right?
Does your patch fix the problem or just the symptom? In other words, if image sizes are being dynamically generated, is the original image going to be shown?
It could be that there is actually a Jetpack issue where where it should be filtering wp_prepare_attachment_for_js to supply all of the added image sizes with URLs off to the WordPress.com endpoints.
#3
@
8 years ago
@aussieguy123 also, if you open up your console, what does _wpCustomizeSettings.controls.site_icon.attachment output? I'm expecting it to have an empty sizes property. But I want to know what the other properties are as well. Please attach the JSON.
#5
@
8 years ago
@westonruter
here is the output of _wpCustomizeSettings.controls.site_icon.attachment . Sizes is actually not empty, but the "full" size is missing.
The fix addresses the symptom. If the root cause is no full size being generated, that should be fixed seperately.
You are right that jetpack is involved. Deactivating jetpack (wp plugin deactivate jetpack --network) works around the problem. The other way around it is to delete the site_icon option ( wp --url=<yoursite.local> option delete site_icon ).
{ "id": 44, "title": "cropped-mars-1.jpg", "filename": "cropped-mars-1.jpg", "url": "http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1.jpg", "link": "http://heraldsun.local/cropped-mars-1-jpg", "alt": "", "author": "1", "description": "http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1.jpg", "caption": "", "name": "cropped-mars-1-jpg", "status": "inherit", "uploadedTo": 0, "date": 1488348427000, "modified": 1488348427000, "menuOrder": 0, "mime": "image/jpeg", "type": "image", "subtype": "jpeg", "icon": "http://heraldsun.local/wp-includes/images/media/default.png", "dateFormatted": "March 1, 2017", "nonces": { "update": "4f78209afe", "delete": "cc6e068550", "edit": "41f6cf352f" }, "editLink": "http://heraldsun.local/wp-admin/post.php?post=44&action=edit", "meta": false, "authorName": "wordpress", "filesizeInBytes": 38387, "filesizeHumanReadable": "37 KB", "height": 512, "width": 512, "orientation": "landscape", "sizes": { "spp_thumbnail": { "height": 75, "width": 100, "url": "http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-100x75.jpg", "orientation": "landscape" }, "promo_thumbnail": { "height": 86, "width": 149, "url": "http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-149x86.jpg", "orientation": "landscape" }, "story_crop": { "height": 237, "width": 316, "url": "http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-316x237.jpg", "orientation": "landscape" }, "story_portrait": { "height": 421, "width": 316, "url": "http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-316x421.jpg", "orientation": "portrait" }, "gallery_portrait": { "height": 488, "width": 366, "url": "http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-366x488.jpg", "orientation": "portrait" }, "gallery_alt_image": { "height": 433, "width": 288, "url": "http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-288x433.jpg", "orientation": "portrait" }, "tablet_thumbnail": { "height": 110, "width": 196, "url": "http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-196x110.jpg", "orientation": "landscape" }, "tablet_square_small": { "height": 234, "width": 234, "url": "http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-234x234.jpg", "orientation": "landscape" } }, "compat": { "item": "<input type=\"hidden\" name=\"attachments[44][menu_order]\" value=\"0\" /><p class=\"media-types media-types-required-info\">Required fields are marked <span class=\"required\">*</span></p>\n\t\t\t<table class=\"compat-attachment-fields\">\t\t<tr class='compat-field-wpcom_thumbnails'>\t\t\t<th scope='row' class='label'><label for='attachments-44-wpcom_thumbnails'><span class='alignleft'>Thumbnail Images</span><br class='clear' /></label></th>\n\t\t\t<td class='field'><p class=\"hide-if-js\">You need to enable Javascript to use this functionality.</p><input type=\"button\" class=\"hide-if-no-js button\" onclick=\"jQuery(this).hide();jQuery('#wpcom-thumbs-44').slideDown('slow');\" value=\"Show Thumbnails\" /><div id=\"wpcom-thumbs-44\" class=\"hidden\"><p>Click on a thumbnail image to modify it. Each thumbnail has likely been scaled down in order to fit nicely into a grid.<br /><strong>Only thumbnails that are cropped are shown.</strong> Other sizes are hidden because they will be scaled to fit.</p><div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=thumbnail\" target=\"_blank\"><strong>thumbnail</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-150x150.jpg\" alt=\"thumbnail\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=spp_thumbnail\" target=\"_blank\"><strong>spp_thumbnail</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-100x75.jpg\" alt=\"spp_thumbnail\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=promo_thumbnail\" target=\"_blank\"><strong>promo_thumbnail</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-149x86.jpg\" alt=\"promo_thumbnail\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=story_crop\" target=\"_blank\"><strong>story_crop</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-316x237.jpg\" alt=\"story_crop\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=story_portrait\" target=\"_blank\"><strong>story_portrait</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-316x421.jpg\" alt=\"story_portrait\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=major_event\" target=\"_blank\"><strong>major_event</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-512x366.jpg\" alt=\"major_event\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=gallery_portrait\" target=\"_blank\"><strong>gallery_portrait</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-366x488.jpg\" alt=\"gallery_portrait\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=gallery_landscape\" target=\"_blank\"><strong>gallery_landscape</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-512x488.jpg\" alt=\"gallery_landscape\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=gallery_landscape_alt\" target=\"_blank\"><strong>gallery_landscape_alt</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-512x433.jpg\" alt=\"gallery_landscape_alt\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=gallery_square\" target=\"_blank\"><strong>gallery_square</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1.jpg\" alt=\"gallery_square\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=venti\" target=\"_blank\"><strong>venti</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1.jpg\" alt=\"venti\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=tablet_landscape\" target=\"_blank\"><strong>tablet_landscape</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1.jpg\" alt=\"tablet_landscape\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=gallery_alt_image\" target=\"_blank\"><strong>gallery_alt_image</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-288x433.jpg\" alt=\"gallery_alt_image\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=tablet_portrait\" target=\"_blank\"><strong>tablet_portrait</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1.jpg\" alt=\"tablet_portrait\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=tablet_square\" target=\"_blank\"><strong>tablet_square</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1.jpg\" alt=\"tablet_square\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=tablet_wide\" target=\"_blank\"><strong>tablet_wide</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1.jpg\" alt=\"tablet_wide\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=tablet_thumbnail\" target=\"_blank\"><strong>tablet_thumbnail</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-196x110.jpg\" alt=\"tablet_thumbnail\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=tablet_gallery_landscape\" target=\"_blank\"><strong>tablet_gallery_landscape</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-512x488.jpg\" alt=\"tablet_gallery_landscape\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=tablet_square_small\" target=\"_blank\"><strong>tablet_square_small</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-234x234.jpg\" alt=\"tablet_square_small\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=spp_thumbnail\" target=\"_blank\"><strong>spp_thumbnail</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-100x75.jpg\" alt=\"spp_thumbnail\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=promo_thumbnail\" target=\"_blank\"><strong>promo_thumbnail</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-149x86.jpg\" alt=\"promo_thumbnail\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=story_crop\" target=\"_blank\"><strong>story_crop</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-316x237.jpg\" alt=\"story_crop\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=story_portrait\" target=\"_blank\"><strong>story_portrait</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-316x421.jpg\" alt=\"story_portrait\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=major_event\" target=\"_blank\"><strong>major_event</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-512x366.jpg\" alt=\"major_event\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=gallery_portrait\" target=\"_blank\"><strong>gallery_portrait</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-366x488.jpg\" alt=\"gallery_portrait\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=gallery_landscape\" target=\"_blank\"><strong>gallery_landscape</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-512x488.jpg\" alt=\"gallery_landscape\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=gallery_landscape_alt\" target=\"_blank\"><strong>gallery_landscape_alt</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-512x433.jpg\" alt=\"gallery_landscape_alt\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=gallery_square\" target=\"_blank\"><strong>gallery_square</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1.jpg\" alt=\"gallery_square\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=venti\" target=\"_blank\"><strong>venti</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1.jpg\" alt=\"venti\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=tablet_landscape\" target=\"_blank\"><strong>tablet_landscape</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1.jpg\" alt=\"tablet_landscape\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=gallery_alt_image\" target=\"_blank\"><strong>gallery_alt_image</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-288x433.jpg\" alt=\"gallery_alt_image\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=tablet_portrait\" target=\"_blank\"><strong>tablet_portrait</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1.jpg\" alt=\"tablet_portrait\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=tablet_square\" target=\"_blank\"><strong>tablet_square</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1.jpg\" alt=\"tablet_square\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=tablet_wide\" target=\"_blank\"><strong>tablet_wide</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1.jpg\" alt=\"tablet_wide\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=tablet_thumbnail\" target=\"_blank\"><strong>tablet_thumbnail</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-196x110.jpg\" alt=\"tablet_thumbnail\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=tablet_gallery_landscape\" target=\"_blank\"><strong>tablet_gallery_landscape</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-512x488.jpg\" alt=\"tablet_gallery_landscape\" /></a></div><div style=\"float:left;margin:0 20px 20px 0;min-width:250px;\"><a href=\"http://heraldsun.local/wp-admin/admin.php?action=wpcom_thumbnail_edit&id=44&size=tablet_square_small\" target=\"_blank\"><strong>tablet_square_small</strong><br /><img src=\"http://heraldsun.local/wp-content/uploads/sites/10/2017/03/cropped-mars-1-234x234.jpg\" alt=\"tablet_square_small\" /></a></div></div></div></td>\n\t\t</tr>\n</table>", "meta": "" } }
#6
@
8 years ago
@aussieguy123 so actually you're not experiencing the issue on WordPress.com but rather on VIP Quickstart?
#7
@
8 years ago
@westonruter I've managed to reproduce this issue on both WordPress.com VIP and also our custom docker environment based on VIP quickstart.
#9
@
8 years ago
A few of our non production servers (also based on vip quickstart) got the issue as well
#10
@
8 years ago
@aussieguy123 I suggest debugging in VIP Quickstart, specifically in this code:
<?php // Disable automatic creation of intermediate images add_filter( 'intermediate_image_sizes', 'wpcom_intermediate_sizes' ); function wpcom_intermediate_sizes ( $sizes ) { if ( ! defined( 'JETPACK_DEV_DEBUG' ) || ! JETPACK_DEV_DEBUG ) { return array(); } return $sizes; }
It's not clear to me if this intermediate_image_sizes filter here is used incorrectly, or if it is being used properly and there is a core bug, in regards to the site icon control.
#11
@
8 years ago
@westonruter I just did some debugging for this jetpack code on my local, after triggering the issue by uploading a site_icon.
Basically the wpcom_intermediate_sizes function takes the sizes and returns them, doesn't do any filtering. I don't think its this function causing the issue.
I still get the issue even if I temporarily delete that function and remove the filter.
#12
@
8 years ago
@aussieguy123 I think 40010.1.diff is a better approach for fixing the symptom you're experiencing. Nevertheless, I am unsure as to why the full size would be undefined. Since this has only been reported in the context of WordPress.com I'd suggest looping in a VIP code wrangler to identify the root cause.
Can you try selectively activating Jetpack modules to identify which module specifically is causing the problem when the plugin is active? I created a Jetpack issue for you and I suggest you follow up there to identify the root cause with the Jetpack team: https://github.com/Automattic/jetpack/issues/6683
Once the root cause is identified, either a fix for Jetpack will ensue and we can close this ticket, or we'll confirm the need for committing a patch for this to core.
#14
@
8 years ago
I can't tell if there's a reason the template uses data.attachment.sizes.full.url instead of data.attachment.url. Looking at https://core.trac.wordpress.org/browser/trunk/src/wp-includes/media.php#L3192, it appears that both will have the same value in this context.
I like attachment:40010.1.diff for the fallback, or even just using data.attachment.url in the first place.
Since it's trivial to remove the full-size, it would be nice if the assumption of its presence wasn't made.
#15
@
8 years ago
WordPress.com VIP found the trigger for this the issue.
We have a plugin which is removing the 'full' size via the 'wp_prepare_attachment_for_js' filter.
This is what triggers the bug. Either my patch or Weston's patch should fix the front end in cases like ours where the 'full' size is removed. I think one should be applied (VIP also suggested this in our support ticket), since its good practice to check if the size exists before using it.
#16
@
8 years ago
- Keywords has-patch added
- Milestone changed from Awaiting Review to 4.7.4
@aussieguy123 Can you provide the source of the wp_prepare_attachment_for_js filter that is being used? I tried acking through vip-wpcom-mu-plugins but couldn't find it.
#17
@
8 years ago
@westonruter its a part of our new authoring plugin. Relevant functions are:
<?php class Image_Crop_Handler { public $wp_image_sizes_to_remove = array( 'thumbnail', 'medium', 'large', 'full' ); /** * Remove default WordPress image sizes. * * @param $attachment * * @filter wp_prepare_attachment_for_js * * @return mixed */ public function remove_non_spp_image_sizes( $attachment ) { $attachment = $this->replace_full_size_label( $attachment ); if ( empty( $attachment['sizes'] ) ) { return $attachment; } foreach ( $attachment['sizes'] as $label => $attributes ) { if ( ! $this->is_valid_spp_image_size( $label, $attributes ) ) { unset( $attachment['sizes'][ $label ] ); } } return $attachment; } /** * Whether the image size is a valid SPP image size or not * * @param $label * @param $attributes * * @return bool */ public function is_valid_spp_image_size( $label, $attributes ) { if ( in_array( $label, $this->wp_image_sizes_to_remove, true ) ) { return false; } if ( isset( $this->image_sizes[ $label ] ) ) { $height = $this->image_sizes[ $label ]['height']; $width = $this->image_sizes[ $label ]['width']; if ( $height !== $attributes['height'] || $width !== $attributes['width'] ) { return false; } } return true; } }
We have some standardized image sizes we use, which is why we remove the default WordPress sizes.
#18
@
8 years ago
- Summary changed from Regression for #36749. Uploading an image into site_icon breaks customizer to Customize: Template for site icon control fails to check if full image size exists before using
- Version changed from trunk to 4.5
#19
@
8 years ago
- Owner set to westonruter
- Resolution set to fixed
- Status changed from new to closed
In 40314:
Note: See
TracTickets for help on using
tickets.
Heres the exact js error:
Uncaught TypeError: Cannot read property 'url' of undefined