the-seo-framework icon indicating copy to clipboard operation
the-seo-framework copied to clipboard

Issue When Cropping Social Image

Open kopepasah opened this issue 8 years ago • 9 comments

When using a plugin which offloads images to an external delivery services (such as S3), the cropping functionality when adding the social images fails.

  • Plugin Used - WP Offload S3, by Delicious Brains.

When the plugin is enable to "remove files from server" the cropping fails, but when files stay on server, cropping succeeds.

Is there a way to make sure cropping works no matter the storage location of the file?

kopepasah avatar Nov 13 '17 06:11 kopepasah

Hi @kopepasah,

Could you elaborate on "cropping fails"? i.e. an error shows up, nothing happens, etc...

sybrew avatar Nov 13 '17 11:11 sybrew

There is a WordPress error thrown in the modal.

My guess is the issue has to do with the file no longer being stored on the server.

kopepasah avatar Nov 15 '17 03:11 kopepasah

Hi @kopepasah

I don't have an access key for Amazon S3, so debugging will be very difficult without further directions. So, could you specify what the error message is?

Also, does image cropping in Customizer work when setting a Logo? TSF uses almost all code of that. image

Thanks! 😄

sybrew avatar Nov 15 '17 07:11 sybrew

Custom logo works fine, but I is seems WP Offload S3 has some custom code to hand copying images back to server when needed (and custom logo action is hooked).

See:

  • https://github.com/deliciousbrains/wp-amazon-s3-and-cloudfront/blob/761e98dad46938e58cf36a85fb536f146a7fe9ea/classes/as3cf-plugin-compatibility.php#L208
  • https://github.com/deliciousbrains/wp-amazon-s3-and-cloudfront/blob/761e98dad46938e58cf36a85fb536f146a7fe9ea/classes/as3cf-plugin-compatibility.php#L374

kopepasah avatar Nov 16 '17 02:11 kopepasah

May be out of scope for this issue, but if I can find some time adding compatibility would be nice.

kopepasah avatar Nov 16 '17 02:11 kopepasah

I think I understand why it doesn't work. Unfortunately, it can't be fixed in this plugin without breaking other plugins.

The reason this error exists is that TSF added its own custom "context" parameter in the wp_ajax saving data. This is to prevent runtime collisions. Now, WP Offload S3 is very strict regarding "context", so TSF's custom one can't be detected without hardcoding it.

WordPress (and TSF) calls this action on crop: wp_ajax_crop_image_pre_save

And it uses these filters:

wp_create_file_in_uploads
wp_ajax_cropped_attachment_metadata
wp_ajax_cropped_attachment_id

I think those are more than enough hooks for this all to work, without "context" checking.

I understand that this requires rescoping in the WP Offload S3 plugin and that it might not be feasible to fix this in the short term; if ever.

Note that this is an edge-case situation, as not many plugins do cropping, and WordPress still handles its cropping code as "private" (i.e. not for plugins/themes); even though it's not marked as such.

I'd love to receive feedback because I can't dig any further than this, and I might be wrong. So, I'll tag one of the lead developers of WP Offload S3: @A5hleyRich

FYI, the code TSF uses to crop images (it's a clone with edits for #40193): https://github.com/sybrew/the-seo-framework/blob/2.9.4/inc/classes/admin-init.class.php#L656-L749

sybrew avatar Nov 16 '17 12:11 sybrew

Yes, it would be great to hear back from the Delicious Brains team.

kopepasah avatar Nov 20 '17 16:11 kopepasah

Any updates on this one? We've just switched over to TSF to test it out, but it's not feasible to tell our clients to click the skip cropping button. Even the ability to disable the cropping entirely would be suitable for our case.

LetterAfterZ avatar Mar 03 '22 04:03 LetterAfterZ

I checked the code, and there is no feasible way to hide the button. Sometimes, a crop is required, for the image would be too large for social media, so I don't think an option to hide it is sensible.

Perhaps, since 5 years, WordPress finally has an API for cropping that allows us to skip using a custom uploading endpoint. Or perhaps, I should copy their filters to the last detail. I will investigate.

However still, back then, when I reported the issue to WordPress, the implementer of the fix didn't understand the problem plugin authors are facing and still messed up the capability checks.

WordPress does not work well with a push-CDN configuration, like Amazon S3. It requires a ton of conditions and checks for every upload endpoint, adds a lot of bloat to your server, and even then gives you no benefit over using a pull-CDN configuration. Pull-CDN plugins like CDN Enabler are about 300 times smaller than Delicious Brains's Offload "Lite". This is because a pull-CDN configuration lets the CDN handle everything automagically, stress-free.

sybrew avatar Mar 03 '22 08:03 sybrew