Visitar URL original
fix: max image dimensions are now configurable and have a higher default by DeepDiver1975 · Pull Request #41193 · owncloud/core · GitHub
Skip to content

fix: max image dimensions are now configurable and have a higher default - #41193

Merged
jnweiger merged 1 commit into
masterfrom
fix/configurable-max-image-dimensions
Feb 23, 2024
Merged

jnweiger merged 1 commit into
masterfrom
fix/configurable-max-image-dimensions

Conversation

@DeepDiver1975

Copy link
Copy Markdown
Contributor

Description

Related Issue

Motivation and Context

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@update-docs

update-docs Bot commented Feb 23, 2024

Copy link
Copy Markdown

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@DeepDiver1975
DeepDiver1975 force-pushed the fix/configurable-max-image-dimensions branch from b880035 to c806846 Compare February 23, 2024 09:22
@jnweiger
jnweiger self-requested a review February 23, 2024 09:48

@jnweiger jnweiger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need to clarify the relation between
preview_max_dimensions and preview_max_x/preview_max_y

Sorry, I did not spot the latter pair earlier.

Comment thread config/config.sample.php
* Default is 6016x4000.
*/
'preview_max_dimensions' => '6016x4000',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A bit further up, we already have this:

/**
 * Define the maximum x-axis width for previews
 * The maximum width, in pixels, of a preview.
 * A value of `null` means there is no limit.
 */
'preview_max_x' => 2048,

/**
 * Define the maximum y-axis width for previews
 * The maximum height, in pixels, of a preview. A value of `null` means there is no limit.
 */
'preview_max_y' => 2048,

That is confusing now. Those values are used in apps/files_sharing/lib/Controllers/ShareController.php and lib/private/Preview.php

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These are output values.
And the new prop defined input values.

But I see your point...... Let me think....

@DeepDiver1975
DeepDiver1975 force-pushed the fix/configurable-max-image-dimensions branch from c806846 to d05c3c9 Compare February 23, 2024 10:39
Comment thread config/config.sample.php Resolved
Comment thread config/config.sample.php Resolved
Comment thread config/config.sample.php Resolved
Comment thread config/config.sample.php Resolved
$maxDimension = \OC::$server->getConfig()->getSystemValue('preview_max_dimensions', '6016x4000');
$exploded = explode('x', strtolower($maxDimension));
if ($exploded === false || \count($exploded) !== 2) {
return [6016, 4000];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code could be refactored so that the default [6016, 4000] only appears once, but up to you.

@jnweiger jnweiger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, thanks for clarification.

@DeepDiver1975
DeepDiver1975 force-pushed the fix/configurable-max-image-dimensions branch from 11f853f to 8f6a51c Compare February 23, 2024 11:26
@sonarqubecloud

Copy link
Copy Markdown

@jnweiger
jnweiger merged commit 536f0b4 into master Feb 23, 2024
@delete-merged-branch
delete-merged-branch Bot deleted the fix/configurable-max-image-dimensions branch February 23, 2024 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[QA] typical Photos from a smartphone are no longer rendered in the builtin image viewer

3 participants