Skip to content
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

Jump to bottom

Non-Integer Thumbnail Sizes Break API Calls #5291

Closed
5 of 10 tasks
v-karbovnichy opened this issue Sep 12, 2022 · 3 comments · Fixed by #5292
Closed
5 of 10 tasks

Non-Integer Thumbnail Sizes Break API Calls #5291

v-karbovnichy opened this issue Sep 12, 2022 · 3 comments · Fixed by #5292
Labels
Status: New

Comments

@v-karbovnichy
Copy link
Contributor

Description of bug

Non-Integer Thumbnail Sizes Break API Calls

Steps to reproduce

  1. Try zooming out several times in your browser.
  2. Reload a DNN Page with the Assets page of the Persona Bar open.
  3. If the thumbnails fail to load, look in the network tab for the failing calls.

It is easy to reproduce the failed API calls with non-integer parameters: make an API call to http://{your dnn install}.dnndev.me/DnnImageHandler.ashx?mode=securefile&fileId={a file id}&MaxWidth=74.98&MaxHeight=42.01

Current behavior

API calls return with HTTP 500

Expected behavior

API parses the non-integer parameters and returns thumbnail based on rounded dimensions

Screenshots

See here https://docs.google.com/document/d/1DgWvWwNloNGleA7zpt6sqb4fT_W1V9gSwzmZhRBWKBg/edit

Error information

n/a

Additional context

n/a

Affected version

  • 10.00.00 alpha build
  • 09.11.00 release candidate
  • 09.10.02 release candidate
  • 09.10.01 latest supported release

Affected browser

  • Chrome
  • Firefox
  • Safari
  • Internet Explorer 11
  • Microsoft Edge (Classic)
  • Microsoft Edge Chromium
@v-karbovnichy
Copy link
Contributor Author

I will submit a PR

@v-karbovnichy v-karbovnichy mentioned this issue Sep 12, 2022
Merged
bdukes pushed a commit that referenced this issue Dec 17, 2022
@v-karbovnichy
Fix for #5291: Non-Integer Thumbnail Sizes Break API Calls (#5292)
00be9a2 
* DNN-61848 - Core Engineering Maintenance

* Update DefaultDimension to be 10 px

Co-authored-by: Ricardo Tejo <ricardo.tejo@trilogy.com>
@DNNMonster
Copy link
Contributor

DNNMonster commented Dec 25, 2022 •
edited
Loading

I am pretty sure this commit introduces a new url requirement for the profilepic mode. If you do not include the maxHeight or maxWidth (which I don't think is currently happening by default) the DefaultDimension of 10px becomes the maxHeight and maxWidth. This makes some renderings blurry and defaults others to small sizes. It could impact themes and modules that use the ImageHandler as well upon the next upgrade.

If there are no additional concerns regarding the maxHeight and maxWidth params then it may be less disruptive to bypass the DefaultDimension implementations for requests made for profilepic mode.

Here is a branch I can submit as a PR that tries to fix this or it may just be referenced on the way to something more robust.

DNNMonster@c74e9eb

zyhfish pushed a commit to zyhfish/Dnn.Platform that referenced this issue Dec 29, 2022
@v-karbovnichy @zyhfish
Fix for dnnsoftware#5291: Non-Integer Thumbnail Sizes Break API Calls (…
4b97a83 
…dnnsoftware#5292)

* DNN-61848 - Core Engineering Maintenance

* Update DefaultDimension to be 10 px

Co-authored-by: Ricardo Tejo <ricardo.tejo@trilogy.com>
@valadas
Copy link
Contributor

valadas commented Jan 5, 2023

@DNNMonster I agree this is a breaking change, can you submit that as a pull request

@DNNMonster DNNMonster mentioned this issue Jan 5, 2023
Merged
@DNNMonster DNNMonster mentioned this issue Feb 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Status: New
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix for #5291: Non-Integer Thumbnail Sizes Break API Calls
3 participants
@valadas @v-karbovnichy @DNNMonster