Skip to content

Improve quality setting handling in Compression settings #978

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

Merged
selul merged 5 commits into from
Oct 9, 2025
Merged

Improve quality setting handling in Compression settings #978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8d4376c
Improve quality setting handling in Compression settings
ineagu Oct 1, 2025
be3acff
fixed linting
ineagu Oct 1, 2025
b0c2099
Update settings.php
ineagu Oct 3, 2025
1fe91ed
fix: handle `mauto` for quality value on saving
Soare-Robert-Daniel Oct 8, 2025
124434d
chore: phpstan
Soare-Robert-Daniel Oct 8, 2025
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
49 changes: 25 additions & 24 deletions assets/src/dashboard/parts/connected/settings/Compression.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,30 @@ const Compression = ({
isSampleLoading,
setIsSampleLoading
}) => {
const getQuality = value => {
if ( 'number' === typeof value ) {
return value;
}

if ( 'auto' === value || 'mauto' === value ) {
return 80;
}

if ( 'high_c' === value ) {
return 90;
}

if ( 'medium_c' === value ) {
return 75;
}

if ( 'low_c' === value ) {
return 55;
}

return 80;
};
Comment on lines +42 to +64

Copilot AI Oct 1, 2025

Copy link

Choose a reason for hiding this comment

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

The toggle handler (lines 106-117) writes 'mauto' into data.quality, but getQuality does not recognize 'mauto', only 'auto', so 'mauto' will fall through to the default branch (80) and any logic expecting a distinct auto sentinel may break or become ambiguous. Either change 'mauto' to 'auto' everywhere or add a condition for 'mauto' in getQuality (and any other downstream logic) to keep behavior consistent.

Copilot uses AI. Check for mistakes.

const {
sampleImages,
isLoading
Expand All @@ -60,6 +84,7 @@ const Compression = ({
const isBestFormatEnabled = 'disabled' !== settings[ 'best_format' ];
const compressionMode = settings[ 'compression_mode' ];
const isRetinaEnabled = 'disabled' !== settings[ 'retina_images' ];

const updateOption = ( option, value ) => {
setCanSave( true );
const data = { ...settings };
Expand All @@ -79,30 +104,6 @@ const Compression = ({
);
};

const getQuality = value => {
if ( 'number' === typeof value ) {
return value;
}

if ( 'auto' === value ) {
return 90;
}

if ( 'high_c' === value ) {
return 90;
}

if ( 'medium_c' === value ) {
return 75;
}

if ( 'low_c' === value ) {
return 55;
}

return 90;
};

const updateQuality = value => {
setCanSave( true );
const data = { ...settings };
Expand Down
10 changes: 7 additions & 3 deletions inc/settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class Optml_Settings {
'compression_mode' => 'custom',
'cloud_sites' => [ 'all' => 'true' ],
'watchers' => '',
'quality' => 'auto',
'quality' => 80,
'wm_id' => - 1,
'wm_opacity' => 1,
'wm_position' => Position::SOUTH_EAST,
Expand Down Expand Up @@ -237,7 +237,9 @@ public function auto_connect() {
* @return array
*/
public function parse_settings( $new_settings ) {
$sanitized = [];
$sanitized = [];
$sanitized_value = '';

foreach ( $new_settings as $key => $value ) {
switch ( $key ) {
case 'admin_bar_item':
Expand Down Expand Up @@ -278,7 +280,9 @@ public function parse_settings( $new_settings ) {
$sanitized_value = $this->to_bound_integer( $value, 100, 5000 );
break;
case 'quality':

Copilot AI Oct 1, 2025

Copy link

Choose a reason for hiding this comment

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

Raising the minimum allowed quality from 1 to 50 is a silent tightening of accepted input that will coerce previously valid stored values (<50) up to 50 on next save, potentially surprising users who intentionally chose lower quality for stronger compression. Consider documenting this change inline (comment) and/or providing a migration note or backward-compat handling (e.g., warning, transitional deprecation) to make the behavioral change explicit.

Suggested change
case 'quality':
case 'quality':
// Minimum allowed quality was raised from 1 to 50.
// Any previously stored values <50 will be coerced to 50 on next save.
// Consider informing users who intentionally chose lower quality for stronger compression.
if (intval($value) < 50) {
// Optional: Log a warning or trigger a deprecation notice for legacy values.
trigger_error(
'The minimum allowed image quality has been raised from 1 to 50. Values below 50 will be set to 50. Please review your settings.',
E_USER_DEPRECATED
);
}

Copilot uses AI. Check for mistakes.
$sanitized_value = $this->to_bound_integer( $value, 1, 100 );
if ( 'mauto' !== $value ) {
$sanitized_value = $this->to_bound_integer( $value, 50, 100 );
}
break;
case 'wm_id':
$sanitized_value = intval( $value );
Expand Down
Loading