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.

Sign up for GitHub

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

Fix generating file names for edited files, and use wp_unique_filename() #23440

Merged
merged 6 commits into from
Jun 26, 2020
Merged

Fix generating file names for edited files, and use wp_unique_filename() #23440

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d816481
Fix generating file names fro edited files, and use wp_unique_filename()
azaozz Jun 25, 2020
6654ec5
Fix cropping when very small changes, show error when saving image th…
azaozz Jun 25, 2020
89f759f
Fix cropping.
azaozz Jun 25, 2020
5e75d21
Fix when to crop test.
azaozz Jun 26, 2020
01e8331
Add 0-100 min/max for the crop values
azaozz Jun 26, 2020
4c245ee
Fix typo.
azaozz Jun 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
186 changes: 124 additions & 62 deletions lib/class-wp-rest-image-editor-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,28 +43,30 @@ public function register_routes() {
'callback' => array( $this, 'apply_edits' ),
'permission_callback' => array( $this, 'permission_callback' ),
'args' => array(
'rotation' => array(
'type' => 'integer',
),

// Crop values are in percents.
'x' => array(
'type' => 'number',
'minimum' => 0,
'required' => true,
'type' => 'number',
'minimum' => 0,
'maximum' => 100,
),
'y' => array(
'type' => 'number',
'minimum' => 0,
'required' => true,
'type' => 'number',
'minimum' => 0,
'maximum' => 100,
),
'width' => array(
'type' => 'number',
'minimum' => 1,
'required' => true,
'type' => 'number',
'minimum' => 0,
'maximum' => 100,
),
'height' => array(
'type' => 'number',
'minimum' => 1,
'required' => true,
),
'rotation' => array(
'type' => 'integer',
'type' => 'number',
'minimum' => 0,
'maximum' => 100,
),
),
),
Expand All @@ -83,7 +85,8 @@ public function register_routes() {
*/
public function permission_callback( $request ) {
if ( ! current_user_can( 'edit_post', $request['media_id'] ) ) {
return new WP_Error( 'rest_cannot_edit_image', __( 'Sorry, you are not allowed to edit images.', 'gutenberg' ), array( 'status' => rest_authorization_required_code() ) );
$error = __( 'Sorry, you are not allowed to edit images.', 'gutenberg' );
return new WP_Error( 'rest_cannot_edit_image', $error, array( 'status' => rest_authorization_required_code() ) );
}

return true;
Expand All @@ -101,90 +104,149 @@ public function permission_callback( $request ) {
public function apply_edits( $request ) {
require_once ABSPATH . 'wp-admin/includes/image.php';

$params = $request->get_params();
$params = $request->get_params();
$attachment_id = $params['media_id'];

$media_id = $params['media_id'];
// This also confirms the attachment is an image.
$image_file = wp_get_original_image_path( $attachment_id );
Copy link
Member

Choose a reason for hiding this comment

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

Might be good to leave a comment saying that this is the original uploaded file.

$image_meta = wp_get_attachment_metadata( $attachment_id );

// Get image information.
$attachment_info = wp_get_attachment_metadata( $media_id );
$media_url = wp_get_attachment_image_url( $media_id, 'original' );
if ( ! $image_meta || ! $image_file ) {
$error = __( 'Unable to get meta information for file.', 'gutenberg' );
return new WP_Error( 'rest_unknown_attachment', $error, array( 'status' => 404 ) );
}

// Check if we need to do anything.
$rotate = 0;
$crop = false;

if ( ! $attachment_info || ! $media_url ) {
return new WP_Error( 'rest_unknown_attachment', __( 'Unable to get meta information for file.', 'gutenberg' ), array( 'status' => 404 ) );
if ( ! empty( $params['rotation'] ) ) {
// Rotation direction: clockwise vs. counter clockwise.
$rotate = 0 - (int) $params['rotation'];
}

$meta = array( 'original_name' => basename( $media_url ) );
if ( isset( $params['x'], $params['y'], $params['width'], $params['height'] ) ) {
$crop = true;
}

if ( isset( $attachment_info['richimage'] ) ) {
$meta = array_merge( $meta, $attachment_info['richimage'] );
if ( ! $rotate && ! $crop ) {
$error = __( 'The image was not erdited. Edit the image before applying the changes.', 'gutenberg' );
azaozz marked this conversation as resolved.
Show resolved Hide resolved
return new WP_Error( 'rest_image_not_edited', $error, array( 'status' => 400 ) );
}

// Try and load the image itself.
$image_path = get_attached_file( $media_id );
if ( empty( $image_path ) ) {
return new WP_Error( 'rest_cannot_find_attached_file', __( 'Unable to find original media file.', 'gutenberg' ), array( 'status' => 500 ) );
$image_editor = wp_get_image_editor( $image_file );

if ( is_wp_error( $image_editor ) ) {
// This image cannot be edited.
$error = __( 'Unable to edit this image.', 'gutenberg' );
return new WP_Error( 'rest_unknown_image_file_type', $error, array( 'status' => 500 ) );
}

$image_editor = wp_get_image_editor( $image_path );
if ( ! $image_editor->load() ) {
azaozz marked this conversation as resolved.
Show resolved Hide resolved
return new WP_Error( 'rest_cannot_load_editor', __( 'Unable to load original media file.', 'gutenberg' ), array( 'status' => 500 ) );
if ( 0 !== $rotate ) {
$result = $image_editor->rotate( $rotate );

if ( is_wp_error( $result ) ) {
$error = __( 'Unable to rotate this image.', 'gutenberg' );
return new WP_Error( 'rest_image_rotation_failed', $error, array( 'status' => 500 ) );
}
}

if ( isset( $params['rotation'] ) ) {
$image_editor->rotate( 0 - $params['rotation'] );
if ( $crop ) {
$size = $image_editor->get_size();

$crop_x = round( ( $size['width'] * floatval( $params['x'] ) ) / 100.0 );
$crop_y = round( ( $size['height'] * floatval( $params['y'] ) ) / 100.0 );
$width = round( ( $size['width'] * floatval( $params['width'] ) ) / 100.0 );
$height = round( ( $size['height'] * floatval( $params['height'] ) ) / 100.0 );

$result = $image_editor->crop( $crop_x, $crop_y, $width, $height );

if ( is_wp_error( $result ) ) {
$error = __( 'Unable to crop this image.', 'gutenberg' );
return new WP_Error( 'rest_image_crop_failed', $error, array( 'status' => 500 ) );
}
}

$size = $image_editor->get_size();
// Calculate the file name.
$image_ext = pathinfo( $image_file, PATHINFO_EXTENSION );
$image_name = wp_basename( $image_file, ".{$image_ext}" );

// Finally apply the modifications.
$crop_x = round( ( $size['width'] * floatval( $params['x'] ) ) / 100.0 );
$crop_y = round( ( $size['height'] * floatval( $params['y'] ) ) / 100.0 );
$width = round( ( $size['width'] * floatval( $params['width'] ) ) / 100.0 );
$height = round( ( $size['height'] * floatval( $params['height'] ) ) / 100.0 );
$image_editor->crop( $crop_x, $crop_y, $width, $height );
// Do not append multiple `-edited` to the file name.
// The user may be editing a previously edited image.
if ( preg_match( '/-edited(-\d+)?$/', $image_name ) ) {
// Remove any `-1`, `-2`, etc. `wp_unique_filename()` will add the proper number.
$image_name = preg_replace( '/-edited(-\d+)?$/', '-edited', $image_name );
azaozz marked this conversation as resolved.
Show resolved Hide resolved
} else {
// Append `-edited` before the extension.
$image_name .= '-edited';
}

// TODO: Generate filename based on edits.
$target_file = 'edited-' . $meta['original_name'];
$filename = "{$image_name}.{$image_ext}";

$filename = rtrim( dirname( $image_path ), '/' ) . '/' . $target_file;
Copy link
Member

Choose a reason for hiding this comment

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

Previously we were overriding the last edited file, right?

Copy link
Contributor Author

@azaozz azaozz Jun 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, kind of. Was pretty messy. Overwriting the last edited file if it is in the same uploads subdirectory, but not all sub-sizes of it. If the edited file was smaller, and not all sub-sizes were created, some of the old sub-sizes were left "orphaned".

This can be fixed but then we will either need to delete the attachment for the previously edited image, or reuse it and replace the image files and meta. Both of these are not good options if the previously edited image was used in a post.

Better workflow and UX would be to keep all edited files, perhaps let the user know they can delete unused images from the media library.

// Create the uploads sub-directory if needed.
$uploads = wp_upload_dir();

// Make the file name unique in the (new) upload directory.
$filename = wp_unique_filename( $uploads['path'], $filename );

// Save to disk.
$saved = $image_editor->save( $filename );
$saved = $image_editor->save( $uploads['path'] . "/$filename" );

if ( is_wp_error( $saved ) ) {
return $saved;
}

// Update attachment details.
// Create new attachment post.
Copy link
Member

Choose a reason for hiding this comment

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

Should we copy the rest of the attachment data?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, may make sense. But then we'll get two (or more) attachments with the same description and caption. In general don't think the image alt text should be carried over as alt has to be contextual. Editing an older, already used image would suggest it is being used in different context. Editing an image just after uploading it won't have alt text.

$attachment_post = array(
'guid' => $saved['path'],
'post_mime_type' => $saved['mime-type'],
'post_title' => pathinfo( $target_file, PATHINFO_FILENAME ),
'guid' => $uploads['url'] . "/$filename",
'post_title' => $filename,
'post_content' => '',
'post_status' => 'inherit',
);

// Add this as an attachment.
$attachment_id = wp_insert_attachment( $attachment_post, $saved['path'], 0, true );
$new_attachment_id = wp_insert_attachment( $attachment_post, $saved['path'], 0, true );

if ( is_wp_error( $attachment_id ) ) {
if ( 'db_update_error' === $attachment_id->get_error_code() ) {
$attachment_id->add_data( array( 'status' => 500 ) );
if ( is_wp_error( $new_attachment_id ) ) {
if ( 'db_update_error' === $new_attachment_id->get_error_code() ) {
$new_attachment_id->add_data( array( 'status' => 500 ) );
} else {
$attachment_id->add_data( array( 'status' => 400 ) );
$new_attachment_id->add_data( array( 'status' => 400 ) );
}

return $new_attachment_id;
}

// Generate image sub-sizes and meta.
$new_image_meta = wp_generate_attachment_metadata( $new_attachment_id, $saved['path'] );

// Copy the EXIF metadata from the original attachment if not generated for the edited image.
if ( ! empty( $image_meta['image_meta'] ) ) {
$empty_image_meta = true;

if ( isset( $new_image_meta['image_meta'] ) && is_array( $new_image_meta['image_meta'] ) ) {
$empty_image_meta = empty( array_filter( array_values( $new_image_meta['image_meta'] ) ) );
}

return $attachment_id;
if ( $empty_image_meta ) {
$new_image_meta['image_meta'] = $image_meta['image_meta'];
}
}

// Generate thumbnails.
$metadata = wp_generate_attachment_metadata( $attachment_id, $saved['path'] );
// Reset orientation. At this point the image is edited and orientation is correct.
if ( ! empty( $new_image_meta['image_meta']['orientation'] ) ) {
$new_image_meta['image_meta']['orientation'] = 1;
}

$metadata['richimage'] = $meta;
// The attachment_id may change if the site is exported and imported.
$new_image_meta['parent_image'] = array(
'attachment_id' => $attachment_id,
// Path to the originally uploaded image file relative to the uploads directory.
'file' => _wp_relative_upload_path( $image_file ),
);

wp_update_attachment_metadata( $attachment_id, $metadata );
wp_update_attachment_metadata( $new_attachment_id, $new_image_meta );

$path = '/wp/v2/media/' . $attachment_id;
$path = '/wp/v2/media/' . $new_attachment_id;
$response = rest_do_request( $path );

if ( ! $response->is_error() ) {
Expand Down
8 changes: 7 additions & 1 deletion packages/block-library/src/image/image-editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,13 @@ export default function ImageEditor( {
function apply() {
setIsProgress( true );

const attrs = crop;
let attrs = {};

// The crop script may return some very small, sub-pixel values when the image was not cropped.
// Crop only when the new size has changed by more than 0.1%.
if ( crop.width < 99.9 || crop.height < 99.9 ) {
attrs = crop;
}

if ( rotation > 0 ) {
attrs.rotation = rotation;
Expand Down