-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add avif support #28
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
base: develop
Are you sure you want to change the base?
Changes from 4 commits
20fc547
66e2886
bb79d6f
e1546b0
d96f055
f6aed4e
2026c9c
4b44ed2
caeba2b
d075d5f
483ac40
1273c4d
e5e0845
f3458cb
09e2a71
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,8 +34,8 @@ function addDropZoneListenerToMediaManager( targetDocument ) { | |
| } | ||
|
|
||
| // Get the file converters for the incoming files. | ||
| const fileConverters = Array.from( event.dataTransfer.files ) | ||
| .map( file => getFileConverter( file ) ) | ||
| const files = Array.from( event.dataTransfer.files ) | ||
| const fileConverters = await Promise.all( files.map( file => getFileConverter( file ) ) ) | ||
|
Comment on lines
+37
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: same
Move Proposed fix (same pattern as select-files.js) const customDropHandler = async event => {
if ( event.__cimo_converted ) {
return
}
const files = Array.from( event.dataTransfer.files )
+
+ // Must stop the event synchronously before any await
+ event.preventDefault()
+ event.stopPropagation()
+
const fileConverters = await Promise.all( files.map( file => getFileConverter( file ) ) )
if ( ! requiresFileConversion( fileConverters ) ) {
+ // No conversion needed — re-dispatch the original drop
+ const dropEvent = new DragEvent( 'drop', { bubbles: true } )
+ Object.defineProperty( dropEvent, 'dataTransfer', {
+ value: event.dataTransfer,
+ writable: false,
+ } )
+ dropEvent.__cimo_converted = true
+ event.target.dispatchEvent( dropEvent )
return
}
...
- event.preventDefault()
- event.stopPropagation()Also applies to: 65-66 🤖 Prompt for AI Agents |
||
|
|
||
| // Do not continue if we do not need to convert any files. | ||
| if ( ! requiresFileConversion( fileConverters ) ) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,8 +34,8 @@ function addSelectFilesListenerToFileUploads( targetDocument ) { | |
| } | ||
|
|
||
| // Get the file converters for the incoming files. | ||
| const fileConverters = Array.from( event.target.files ) | ||
| .map( file => getFileConverter( file ) ) | ||
| const files = Array.from( event.target.files ) | ||
| const fileConverters = await Promise.all( files.map( file => getFileConverter( file ) ) ) | ||
|
Comment on lines
+37
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: After the Before this PR Proposed fix const selectFilesListener = async event => {
// Check if it's a file select.
if ( event.target.type !== 'file' ) {
return
}
// If this is a synthetic change event dispatched by us after conversion, skip conversion.
if ( event.__cimo_converted ) {
return
}
// Get the file converters for the incoming files.
const files = Array.from( event.target.files )
+
+ // Must stop the event synchronously before any await,
+ // otherwise the browser will finish dispatching the event
+ // while we're waiting for the async format-support check.
+ event.preventDefault()
+ event.stopPropagation()
+ event.stopImmediatePropagation()
+
const fileConverters = await Promise.all( files.map( file => getFileConverter( file ) ) )
// Do not continue if we do not need to convert any files.
if ( ! requiresFileConversion( fileConverters ) ) {
+ // No conversion needed — re-dispatch the original event
+ const changeEvent = new Event( 'change', { bubbles: true } )
+ changeEvent.__cimo_converted = true
+ event.target.dispatchEvent( changeEvent )
return
}
...
- // Prevent the default file handling
- event.preventDefault()
- event.stopPropagation()
- event.stopImmediatePropagation()Note: The same issue exists in Also applies to: 64-66 🤖 Prompt for AI Agents |
||
|
|
||
| // Do not continue if we do not need to convert any files. | ||
| if ( ! requiresFileConversion( fileConverters ) ) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,12 @@ | ||
| import { Converter } from './converter-abstract' | ||
| import { isFormatSupported } from './util' | ||
|
|
||
| // Supported output formats | ||
| const supportedFormats = [ | ||
| { value: 'webp', mimeType: 'image/webp' }, | ||
| { value: 'jpg', mimeType: 'image/jpeg' }, | ||
| { value: 'png', mimeType: 'image/png' }, | ||
| { value: 'avif', mimeType: 'image/avif' }, | ||
| ] | ||
|
|
||
| /** | ||
|
|
@@ -19,6 +21,7 @@ class ImageConverter extends Converter { | |
| 'image/png', | ||
| 'image/webp', | ||
| 'image/jpg', | ||
| 'image/avif', | ||
| ] | ||
| } | ||
|
|
||
|
|
@@ -171,24 +174,37 @@ class ImageConverter extends Converter { | |
|
|
||
| const format = supportedFormats.find( f => f.value === outputFormat ) | ||
| // Only use quality for lossy formats | ||
| const q = ( outputFormat === 'webp' || outputFormat === 'jpg' ) ? quality : undefined | ||
|
|
||
| canvas.toBlob( function( blob ) { | ||
| // Clean up resources | ||
| URL.revokeObjectURL( objectUrl ) | ||
| objectUrl = null | ||
|
|
||
| // Clear canvas to free memory | ||
| ctx.clearRect( 0, 0, canvas.width, canvas.height ) | ||
| canvas.width = 0 | ||
| canvas.height = 0 | ||
|
|
||
| if ( blob ) { | ||
| resolve( blob ) | ||
| } else { | ||
| reject( new Error( 'Failed to convert image' ) ) | ||
| const q = ( outputFormat === 'webp' || outputFormat === 'jpg' || outputFormat === 'avif' ) ? quality : undefined | ||
|
|
||
| if ( outputFormat === 'avif' ) { | ||
| try { | ||
| const { encode: encodeAvif } = await import( '@jsquash/avif' ) | ||
| const imageData = ctx.getImageData( 0, 0, width, height ) | ||
| const avifQuality = Math.max( 1, Math.min( 100, Math.round( ( ( q ?? 0.5 ) * 100 ) ) ) ) | ||
| const avifBuffer = await encodeAvif( imageData, { quality: avifQuality } ) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result:
Practical mapping (if you have a 0–100 “quality” and want a // quality: 0..100 where 100 ~= best
const cqLevel = Math.round((100 - quality) * 63 / 100);(This mapping is approximate; different encoders/settings can shift results.) Sources: [1] Fix AVIF quality parameter: replace
Map quality (0–1) to 🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://github.com/jamsinclair/jSquash/blob/main/packages/avif/codec/enc/avif_enc.d.ts#L7 @jsquash/avif's encode has a quality parameter There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| const avifBlob = new Blob( [ avifBuffer ], { type: 'image/avif' } ) | ||
| resolve( avifBlob ) | ||
| } catch ( e ) { | ||
| reject( new Error( `Failed to encode AVIF: ${ e instanceof Error ? e.message : 'Unknown error' }` ) ) | ||
| } | ||
| }, format.mimeType, q ) | ||
| } else { | ||
| canvas.toBlob( function( blob ) { | ||
| // Clean up resources | ||
| URL.revokeObjectURL( objectUrl ) | ||
| objectUrl = null | ||
|
|
||
| // Clear canvas to free memory | ||
| ctx.clearRect( 0, 0, canvas.width, canvas.height ) | ||
| canvas.width = 0 | ||
| canvas.height = 0 | ||
|
|
||
| if ( blob ) { | ||
| resolve( blob ) | ||
| } else { | ||
| reject( new Error( 'Failed to convert image' ) ) | ||
| } | ||
| }, format.mimeType, q ) | ||
| } | ||
| } | ||
|
|
||
| img.onerror = () => { | ||
|
|
@@ -225,8 +241,7 @@ class ImageConverter extends Converter { | |
| } | ||
|
|
||
| // Check if the browser supports the desired output format | ||
| const testCanvas = document.createElement( 'canvas' ) | ||
| if ( formatInfo && ! testCanvas.toDataURL( formatInfo.mimeType ).startsWith( `data:${ formatInfo.mimeType }` ) ) { | ||
| if ( ! await isFormatSupported( format ) ) { | ||
| // If not supported, skip conversion and return the original file | ||
| // eslint-disable-next-line no-console | ||
| console.error( '[Cimo] ' + format + ' is not supported by the browser, please use another modern browser' ) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restrict
image_output_formatto an allowlist of valid values.Currently the schema has no
enumconstraint and the sanitizer only callssanitize_text_field, meaning any arbitrary string (including malicious input) can be persisted. Since only'webp'and'avif'are valid, add validation at both layers:Proposed fix
In the REST schema (around line 59):
'image_output_format' => [ 'type' => 'string', + 'enum' => [ 'webp', 'avif' ], ],In
sanitize_options(around line 225):if ( isset( $options['image_output_format'] ) ) { - $sanitized['image_output_format'] = sanitize_text_field( $options['image_output_format'] ); + $format = sanitize_text_field( $options['image_output_format'] ); + $sanitized['image_output_format'] = in_array( $format, [ 'webp', 'avif' ], true ) ? $format : 'webp'; }Also applies to: 224-227
🤖 Prompt for AI Agents