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.

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

Previews sending the mimetype icon have a wrong size #18655

Closed
nickvergessen opened this issue Aug 29, 2015 · 10 comments
Closed

Previews sending the mimetype icon have a wrong size #18655

nickvergessen opened this issue Aug 29, 2015 · 10 comments

Comments

@nickvergessen
Copy link
Contributor

nickvergessen commented Aug 29, 2015

  1. Upload an empty text file
  2. Open the preview:
    http://localhost/index.php/core/preview.png?file=/test.txt&x=150&y=150

Expected

The preview is 150*150 pixels

Actual

The preview is 16*16pixels

Analysis

\OC_Preview::getMimeIcon() does not call the resize methods, it just changes the source.

@oparoz @georgehrke

@georgehrke
Copy link
Contributor

@oparoz IIRC You modified the mimetype-fallback to use a centered 16x16 icon, right?

The preview is 150*150 pixels

We can't scale up the svg, because that would introduce a hard-dependency on imagick. Scaling up the png is not good.

This is kind of part of the discussion, whether or not the preview api should return mimetype-icons at all (which it should not IMHO) or if it should just send some error code instead. In the case of an error code, the js should take care of using the svg mime type icon instead.

@nickvergessen
Copy link
Contributor Author

Well the problem is, activity app already checks whether the mimetype is supported, to avoid this request.

Oh, I just discovered the isAvailable() method, so I can use that, it will take away the blurriness, but still the size is smaller.

@jancborchardt
Copy link
Member

Do I need to fix anything here with the filetype icons? I hope not right, because previously it was generated from the SVG up to 150*150px, right?

@rullzer
Copy link
Contributor

rullzer commented Aug 30, 2015

This should be fixed once #18675 is merged.
I removed the returning of the icon there since I agree with @oparoz the current behaviour is weird.

Also now that we have the mimetype js stuff there is no need for it.

@oparoz
Copy link
Contributor

oparoz commented Sep 3, 2015

OK, so 3 things need to happen if we want to fix this in 8.2:

  1. Re-open @rullzer's core/ajax/preview.php to controller #18675 or add an additional check to the ajax endpoint so that scripts get the proper status code
  2. Fix all the core methods which query that endpoint so that they use the mimetype.js SVG fallback. (Make sure IE8 gets its nice, stretched 150x150 PNG of the 16x16 icon)
  3. Remove the mimeicon fallback from preview (if the mobile team is OK with that cc @davivel)

@davivel
Copy link

davivel commented Sep 7, 2015

For mobile apps we prefer error code. Removing MIME icon fallback is fine for us.

Thanks for the ping :)

@ghost ghost modified the milestones: 8.2.1-next-maintenance, 8.2-current Sep 23, 2015
@ghost
Copy link

ghost commented Oct 25, 2015

it looks like we won't be fixing this until 9.0

@ghost ghost modified the milestones: 9.0-current, 8.2.1-current-maintenance Oct 25, 2015
@ghost ghost modified the milestones: 9.1-next, 9.0-current Feb 22, 2016
@ghost ghost added the old-inactive label Feb 22, 2016
@PVince81 PVince81 modified the milestones: 9.1, 9.1.1 Jul 18, 2016
@PVince81 PVince81 modified the milestones: 9.1.2, 9.1.1 Sep 21, 2016
@PVince81 PVince81 modified the milestones: 9.1.3, 9.1.2 Oct 20, 2016
@PVince81 PVince81 modified the milestones: 9.2, 9.1.3 Nov 30, 2016
@PVince81 PVince81 modified the milestones: backlog, 10.0 Apr 7, 2017
@PVince81
Copy link
Contributor

PVince81 commented Apr 7, 2017

backlog for now, not critical (please tell if you think otherwise)

@ownclouders
Copy link
Contributor

Hey, this issue has been closed because the label status/STALE is set and there were no updates for 7 days. Feel free to reopen this issue if you deem it appropriate.

(This is an automated comment from GitMate.io.)

@lock
Copy link

lock bot commented Jul 31, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants