Skip to content

fix: allow adding an error note in media library and metabox #33

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

Open
Arukuen wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix: allow adding an error note in media library and metabox #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 17 additions & 3 deletions src/admin/class-meta-box.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,23 @@ function cimo_get_media_type_label( $mimetype ) {

$cimo = $metadata['cimo'];

$converted_format_raw = isset( $cimo['convertedFormat'] ) ? $cimo['convertedFormat'] : ( isset( $post->post_mime_type ) ? $post->post_mime_type : '' );
$converted_format = $converted_format_raw ? cimo_convert_mimetype_to_format( $converted_format_raw ) : '';
$media_type_label = cimo_get_media_type_label( $converted_format_raw );

if ( isset( $cimo['errorName'] ) ) {
echo '<div class="cimo-media-manager-metadata-title-container">';
echo '<h3 class="cimo-media-manager-metadata-title">' . sprintf( esc_html__( '%s Conversion Skipped', 'cimo-image-optimizer' ), esc_html( $media_type_label ) ) . '</h3>';
echo '</div>';

if ( $cimo['errorName'] === 'FileSizeExceededError' ) {
echo'<p>The WebP version of this image was larger than the original, so the original file was kept instead.</p>';
echo '<p>You can try lowering the image quality setting to generate a smaller WebP file.</p>';
echo '<p>Change the file type if needed.</p>';
}
return;
}
Comment on lines +88 to +103
Copy link

@coderabbitai coderabbitai bot Mar 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Error handling path needs internationalization.

The error messages on lines 98-100 are hardcoded in English, unlike other strings in this file that use esc_html__(). For consistency with WordPress i18n practices, these should be translatable.

Also, line 98 has a minor syntax issue: missing space after echo.

🌐 Proposed fix for i18n and formatting 
 						if ( $cimo['errorName'] === 'FileSizeExceededError' ) {
-							echo'<p>The WebP version of this image was larger than the original, so the original file was kept instead.</p>';
-							echo '<p>You can try lowering the image quality setting to generate a smaller WebP file.</p>';
-							echo '<p>Change the file type if needed.</p>';
+							echo '<p>' . esc_html__( 'The WebP version of this image was larger than the original, so the original file was kept instead.', 'cimo-image-optimizer' ) . '</p>';
+							echo '<p>' . esc_html__( 'You can try lowering the image quality setting to generate a smaller WebP file.', 'cimo-image-optimizer' ) . '</p>';
+							echo '<p>' . esc_html__( 'Change the file type if needed.', 'cimo-image-optimizer' ) . '</p>';
 						}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/admin/class-meta-box.php` around lines 88 - 103, Replace the three
hardcoded English paragraphs inside the FileSizeExceededError branch (the echo
statements that output the explanatory <p> lines) with translatable, escaped
strings using esc_html__( ..., 'cimo-image-optimizer' ) and concatenate them
into the echo calls (e.g., echo '<p>' . esc_html__( 'The WebP version of this
image was larger than the original, so the original file was kept instead.',
'cimo-image-optimizer' ) . '</p>';), ensure you add the missing space after each
echo, and keep existing context (the FileSizeExceededError check within the
block that uses $cimo and $media_type_label such as cimo_get_media_type_label).


// Calculate optimization savings
$compression_savings = isset( $cimo['compressionSavings'] ) ? floatval( $cimo['compressionSavings'] ) : null;
$optimization_savings = $compression_savings !== null ? number_format( 100 - ( $compression_savings * 100 ), 2 ) : null;
Expand All @@ -96,9 +113,6 @@ function cimo_get_media_type_label( $mimetype ) {
$original_size = cimo_format_filesize( $original_filesize );
$converted_size = cimo_format_filesize( $converted_filesize );

$converted_format_raw = isset( $cimo['convertedFormat'] ) ? $cimo['convertedFormat'] : ( isset( $post->post_mime_type ) ? $post->post_mime_type : '' );
$converted_format = $converted_format_raw ? cimo_convert_mimetype_to_format( $converted_format_raw ) : '';
$media_type_label = cimo_get_media_type_label( $converted_format_raw );
$converttime = isset( $cimo['conversionTime'] ) ? floatval( $cimo['conversionTime'] ) : null;
if ( $converttime !== null ) {
if ( $converttime < 1000 ) {
Expand Down
3 changes: 3 additions & 0 deletions src/admin/class-metadata.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public function register_rest_route() {
'convertedFilesize',
'conversionTime',
'compressionSavings',
'errorName'
];
if ( ! is_array( $value ) ) {
// translators: The %s is the parameter name.
Expand Down Expand Up @@ -85,6 +86,7 @@ public function register_rest_route() {
'convertedFilesize',
'conversionTime',
'compressionSavings',
'errorName'
];
$sanitized = [];
if ( is_array( $value ) ) {
Expand Down Expand Up @@ -137,6 +139,7 @@ public function save_metadata( $request ) {
'convertedFilesize',
'conversionTime',
'compressionSavings',
'errorName'
];
$sanitized_metadata = [];
foreach ( $metadata_array as $item ) {
Expand Down
6 changes: 6 additions & 0 deletions src/admin/css/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,14 @@
margin-top: -1px;
fill: #19af19;
}

svg.error {
fill: #b31515;
}
}



.cimo-media-manager-metadata ul {
list-style: none;
padding: 0;
Expand Down
12 changes: 11 additions & 1 deletion src/admin/js/media-manager/drop-zone.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,17 @@ function addDropZoneListenerToMediaManager( targetDocument ) {
hasError = true
// eslint-disable-next-line no-console
console.warn( error )
return { file: converter.file, metadata: null }

// Add the error name in the metadata for selected error for displaying
// as a note.
const metadata = error.isDisplayNote ? {
filename: converter.file.name,
errorName: error.name,
} : null

return {
file: converter.file, metadata,
}
}
} )
)
Expand Down
12 changes: 11 additions & 1 deletion src/admin/js/media-manager/select-files.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,17 @@ function addSelectFilesListenerToFileUploads( targetDocument ) {
hasError = true
// eslint-disable-next-line no-console
console.warn( error )
return { file: converter.file, metadata: null }

// Add the error name in the metadata for selected error for displaying
// as a note.
const metadata = error.isDisplayNote ? {
filename: converter.file.name,
errorName: error.name,
} : null

return {
file: converter.file, metadata,
}
}
} )
)
Expand Down
119 changes: 68 additions & 51 deletions src/admin/js/media-manager/sidebar-info.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,73 +93,90 @@ function injectCimoMetadata( {
const convertedFormatRaw = customMetadata.convertedFormat || model.get( 'mime' ) || ''
const mediaTypeLabel = getMediaTypeLabel( convertedFormatRaw )

let html = `
let html = ''

if ( customMetadata.errorName ) {
html = `
<div class="cimo-media-manager-metadata-title-container">
<h3 class="cimo-media-manager-metadata-title">
${ escape( sprintf( __( '%s Conversion Skipped', 'cimo-image-optimizer' ), mediaTypeLabel ) ) }
</h3>
</div>
`
if ( customMetadata.errorName === 'FileSizeExceededError' ) {
html += `
<p>The WebP version of this image was larger than the original, so the original file was kept instead.</p>
<p>You can try lowering the image quality setting to generate a smaller WebP file.</p>
<p>Change the file type if needed.</p>
`
}
} else {
html = `
<div class="cimo-media-manager-metadata-title-container">
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 672 672" height="20" width="20">
<path d="M132.5 132.5C182.4 82.5 253 56 336 56C419 56 489.6 82.5 539.5 132.5C589.4 182.5 616 253 616 336C616 419 589.5 489.6 539.5 539.5C489.5 589.4 419 616 336 616C253 616 182.4 589.5 132.5 539.5C82.6 489.5 56 419 56 336C56 253 82.5 182.4 132.5 132.5z"/>
</svg>
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 672 672" height="20" width="20"><path d="M132.5 132.5C182.4 82.5 253 56 336 56C419 56 489.6 82.5 539.5 132.5C589.4 182.5 616 253 616 336C616 419 589.5 489.6 539.5 539.5C489.5 589.4 419 616 336 616C253 616 182.4 589.5 132.5 539.5C82.6 489.5 56 419 56 336C56 253 82.5 182.4 132.5 132.5zM465.5 273.9C477.6 264.2 479.5 246.6 469.9 234.5C460.3 222.4 442.6 220.5 430.5 230.1C378 272.1 330.3 341.9 306.7 379.4C291.4 359.3 267.2 331.1 239.5 312.6C226.6 304 209.2 307.5 200.7 320.4C192.2 333.3 195.6 350.7 208.5 359.2C237.4 378.5 264.1 415.1 274.1 429.9C281.5 440.9 294 447.9 307.9 447.9C322.3 447.9 335.5 440.3 342.8 428C357.2 403.5 410 318.3 465.6 273.8z"/></svg>
<h3 class="cimo-media-manager-metadata-title">
${ escape( sprintf( __( '%s Optimized by Cimo', 'cimo-image-optimizer' ), mediaTypeLabel ) ) }
</h3>
</div>
<ul>
`

const optimizationSavings = customMetadata.compressionSavings
? ( 100 - ( customMetadata.compressionSavings * 100 ) ).toFixed( 2 )
: null
const optimizationSavings = customMetadata.compressionSavings
? ( 100 - ( customMetadata.compressionSavings * 100 ) ).toFixed( 2 )
: null

const kbSaved = formatFilesize(
customMetadata.originalFilesize - customMetadata.convertedFilesize,
1,
true
)
const kbSaved = formatFilesize(
customMetadata.originalFilesize - customMetadata.convertedFilesize,
1,
true
)

const optimizationSavingsClass =
const optimizationSavingsClass =
optimizationSavings > 0
? 'cimo-optimization-savings-up'
: 'cimo-optimization-savings-down'

html += `
<li class="cimo-compression-savings ${ escape( optimizationSavingsClass ) }">
Saved ${ escape( optimizationSavings ) }%
<span class="cimo-compression-savings-bytes">(${ escape( kbSaved ) })</span>
</li>
`

html += `
<li class="cimo-filesize-original">
Original: <span class="cimo-value">${ escape( formatFilesize( parseInt( customMetadata.originalFilesize ) || 0 ) ) }</span>
</li>
<li class="cimo-filesize-optimized">
Optimized: <span class="cimo-value">${ escape( formatFilesize( parseInt( customMetadata.convertedFilesize ) || 0 ) ) }</span>
</li>
`

html += `
<li class="cimo-converted">
🏞️ Converted to <span class="cimo-value">${ escape( convertMimetypeToFormat( customMetadata.convertedFormat ) ) }</span>
</li>
`

let conversionTimeDisplay = 'N/A'
if ( customMetadata.conversionTime ) {
const timeMs = parseFloat( customMetadata.conversionTime )
conversionTimeDisplay =
timeMs < 1000
? `${ timeMs.toFixed( 0 ) } ms`
: timeMs < 60000
? `${ ( timeMs / 1000 ).toFixed( 1 ) } sec`
: `${ ( timeMs / 60000 ).toFixed( 1 ) } min`
html += `
<li class="cimo-compression-savings ${ escape( optimizationSavingsClass ) }">
Saved ${ escape( optimizationSavings ) }%
<span class="cimo-compression-savings-bytes">(${ escape( kbSaved ) })</span>
</li>
`

html += `
<li class="cimo-filesize-original">
Original: <span class="cimo-value">${ escape( formatFilesize( parseInt( customMetadata.originalFilesize ) || 0 ) ) }</span>
</li>
<li class="cimo-filesize-optimized">
Optimized: <span class="cimo-value">${ escape( formatFilesize( parseInt( customMetadata.convertedFilesize ) || 0 ) ) }</span>
</li>
`

html += `
<li class="cimo-converted">
🏞️ Converted to <span class="cimo-value">${ escape( convertMimetypeToFormat( customMetadata.convertedFormat ) ) }</span>
</li>
`

let conversionTimeDisplay = 'N/A'
if ( customMetadata.conversionTime ) {
const timeMs = parseFloat( customMetadata.conversionTime )
conversionTimeDisplay =
timeMs < 1000
? `${ timeMs.toFixed( 0 ) } ms`
: timeMs < 60000
? `${ ( timeMs / 1000 ).toFixed( 1 ) } sec`
: `${ ( timeMs / 60000 ).toFixed( 1 ) } min`
}

html += `
<li class="cimo-time">
⚡️ Done in <span class="cimo-value">${ escape( conversionTimeDisplay ) }</span>
</li>
</ul>
`
}

html += `
<li class="cimo-time">
⚡️ Done in <span class="cimo-value">${ escape( conversionTimeDisplay ) }</span>
</li>
</ul>
`

customContent.innerHTML = html
container.appendChild( customContent )
}
Expand Down
10 changes: 7 additions & 3 deletions src/shared/converters/image-converter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,11 @@ class ImageConverter extends Converter {
const end = performance.now()

// If the resulting image is bigger than the input, return the original file unchanged.
if ( convertedBlob.size > file.size ) {
throw new Error( `Resulting image is bigger than the input, skipping conversion.` )
if ( convertedBlob.size < file.size ) {
const error = new Error( 'Resulting image is bigger than the input, skipping conversion.' )
error.name = 'FileSizeExceededError'
error.isDisplayNote = true
throw error
}
Comment on lines 278 to 284
Copy link

@coderabbitai coderabbitai bot Mar 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical bug: Size comparison logic is inverted.

The condition convertedBlob.size < file.size throws an error when the converted file is smaller than the original, which is the opposite of the intended behavior. According to the PR objectives and the comment on line 278, the error should be thrown when the converted file is larger than the original.

🐛 Proposed fix 
 			// If the resulting image is bigger than the input, return the original file unchanged.
-			if ( convertedBlob.size < file.size ) {
+			if ( convertedBlob.size > file.size ) {
 				const error = new Error( 'Resulting image is bigger than the input, skipping conversion.' )
 				error.name = 'FileSizeExceededError'
 				error.isDisplayNote = true
 				throw error
 			}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// If the resulting image is bigger than the input, return the original file unchanged.
if ( convertedBlob.size > file.size ) {
throw new Error( `Resulting image is bigger than the input, skipping conversion.` )
if ( convertedBlob.size < file.size ) {
const error = new Error( 'Resulting image is bigger than the input, skipping conversion.' )
error.name = 'FileSizeExceededError'
error.isDisplayNote = true
throw error
}
// If the resulting image is bigger than the input, return the original file unchanged.
if ( convertedBlob.size > file.size ) {
const error = new Error( 'Resulting image is bigger than the input, skipping conversion.' )
error.name = 'FileSizeExceededError'
error.isDisplayNote = true
throw error
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/shared/converters/image-converter.js` around lines 278 - 284, The
size-comparison is inverted: currently it throws when convertedBlob.size <
file.size; change the condition to throw only when convertedBlob.size >
file.size so that the FileSizeExceededError (error.name =
'FileSizeExceededError', error.isDisplayNote = true) is raised when the
converted image is larger than the original, keeping the existing error message
and behavior in the image conversion logic that uses convertedBlob and file.


// Get the file extension for the new format
Expand Down Expand Up @@ -306,7 +309,8 @@ class ImageConverter extends Converter {

return { file: outFile, metadata: conversionMetadata }
} catch ( error ) {
throw new Error( `Failed to convert image: ${ error.message }` )
error.message = `Failed to convert image: ${ error.message }`
throw error
}
}
}
Expand Down