Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Back up edited full image sources when restoring the original image #314

Merged
merged 10 commits into from
Jun 16, 2022
Prev Previous commit
Next Next commit
Remove support for IMAGE_EDIT_OVERWRITE constant.
Don't add support for the constant that exists in core and
has not been fully supported here, all changes and behavior
for `IMAGE_EDIT_OVERWRITE` would be addressed later.
mitogh committed Jun 5, 2022
commit 178cdbaeb046839cc70e73ee3cfe978bf769be1f
39 changes: 19 additions & 20 deletions modules/images/webp-uploads/image-edit.php
Original file line number Diff line number Diff line change
@@ -395,30 +395,29 @@ function webp_uploads_restore_image( $attachment_id, $data ) {
$backup_sources = array();
felixarntz marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the same question as Felix and would like to explore that edge case.

I tried this edge case in which the _wp_attachment_backup_sources meta value is null or not an array. As per the below code snippet, it will initialize an empty array for $backup_sources.

if ( ! is_array( $backup_sources ) ) {
	$backup_sources = array();
}

After this snippet, we have another snippet that checks two things if full-orig key is not set in array $backup_sources or it's not an array.

if ( ! isset( $backup_sources['full-orig'] ) || ! is_array( $backup_sources['full-orig'] ) ) {
	return $data;
}

In this edge case the function return $data so as I understood it is worth initializing an empty array instead of returning the $data as pervious it does.

Please let me know if I'm getting anything wrong.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can tell it doesn't really matter though which way to go about this line of code, for the following reason:

  • If $backup_sources is not an array, the following condition ! isset( $backup_sources['full-orig'] ) || ! is_array( $backup_sources['full-orig'] ) will also fore sure evaluate to false.
  • So we can either initialize $backup_sources as empty array, causing $data to be returned in the following clause, or we can return $data right away. I don't think it changes the function behavior at all honestly.

cc @mitogh

}

if ( ! defined( 'IMAGE_EDIT_OVERWRITE' ) || ! IMAGE_EDIT_OVERWRITE ) {
$suffix = null;
foreach ( $data['sources'] as $mime_type => $properties ) {
if ( empty( $properties['file'] ) ) {
continue;
}

$matches = array();
if ( ! preg_match( '/-e(\d{13})/', $properties['file'], $matches ) ) {
continue;
}

$suffix = $matches[1];
break;
// TODO: Add support for constant. `IMAGE_EDIT_OVERWRITE`.
$suffix = null;
foreach ( $data['sources'] as $mime_type => $properties ) {
if ( empty( $properties['file'] ) ) {
continue;
}

if ( null === $suffix ) {
$suffix = 'orig';
$matches = array();
if ( ! preg_match( '/-e(\d{13})/', $properties['file'], $matches ) ) {
continue;
}

if ( empty( $backup_sources[ "full-{$suffix}" ] ) ) {
$backup_sources[ "full-{$suffix}" ] = $data['sources'];
update_post_meta( $attachment_id, '_wp_attachment_backup_sources', $backup_sources );
}
$suffix = $matches[1];
break;
}

if ( null === $suffix ) {
$suffix = 'orig';
}

if ( empty( $backup_sources[ "full-{$suffix}" ] ) ) {
$backup_sources[ "full-{$suffix}" ] = $data['sources'];
update_post_meta( $attachment_id, '_wp_attachment_backup_sources', $backup_sources );
}

if ( ! isset( $backup_sources['full-orig'] ) || ! is_array( $backup_sources['full-orig'] ) ) {