fix ImageManager::compressUntilSize() - #2200
Open
blued-gear wants to merge 4 commits into
Open
Conversation
blued-gear
commented
Aug 7, 2026
| public function compressUntilSize(string $filePath, string $extension, int $maxBytes): bool | ||
| { | ||
| if (-1 === $this->imageCompressionQuality || filesize($filePath) <= $maxBytes) { | ||
| if (-1 === (int)$this->imageCompressionQuality || filesize($filePath) <= $maxBytes) { |
Collaborator
Author
There was a problem hiding this comment.
IDE showed me that this comparison would always fail as -1 is an int and imageCompressionQuality is a float
blued-gear
commented
Aug 7, 2026
| $quality = $this->imageCompressionQuality; | ||
| } | ||
| while ($bytes > $maxBytes && $quality > 0.1) { | ||
| while ($bytes > $maxBytes && $quality > 0.3) { |
Collaborator
Author
There was a problem hiding this comment.
I changed it to 0.3 to shorten the loop. IMO even at this quality level the images would be well fried already.
BentiGorlich
approved these changes
Aug 18, 2026
BentiGorlich
left a comment
BentiGorlich
left a comment
Member
There was a problem hiding this comment.
I am approving this, but have you though about adding a test? If this code might be flawed from the beginning, maybe that would be a good idea
| public function compressUntilSize(string $filePath, string $extension, int $maxBytes): bool | ||
| { | ||
| if (-1 === $this->imageCompressionQuality || filesize($filePath) <= $maxBytes) { | ||
| if (-1 === (int) $this->imageCompressionQuality || filesize($filePath) <= $maxBytes) { |
Member
There was a problem hiding this comment.
if you put -1 in the env file, I believe it might be converted to an int? At least that was my idea
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The loop checks if the compression had any effect, but it does so with the original file size. If the image got compressed in the first iteration, the condition then would always be false.