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

app.getFileIcon() does not return the expected file icon #15809

Closed
oliverschwendener opened this issue Nov 22, 2018 · 11 comments
Closed

app.getFileIcon() does not return the expected file icon #15809

oliverschwendener opened this issue Nov 22, 2018 · 11 comments

Comments

@oliverschwendener
Copy link

I'm trying to get the native file icons on macOS and Windows. When I use app.getFileIcon() I don't get the expected icons. For example on macOS (using TypeScript):

const filePath = path.normalize("/Applications/Adobe Photoshop CC 2019.app");

app.getFileIcon(filePath, { size: "normal" }, (err: Error, icon: NativeImage) => {
    writeFileSync("icon.png", icon.toPNG());
});

What I expected:

image

What I got:

image

I tried using other 3rd party libraries to extract the native icons

but the problems I ran into were:

  • no TypeScript support
  • buggy
  • only work on a specific operating system
  • not maintained anymore
  • I would really like to use electron's build in APIs :)

I read this conversation and saw that I have to normalize the file path but it didn't help. Is there a way to optimize this API to get the file icon the operating system's file browser displays? And maybe also support icons lager than 32x32?

Info

  • macOS 10.13.6
  • electron 2.0.8

Cheers

@welcome
Copy link

welcome bot commented Nov 22, 2018

👋 Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

To help make it easier for us to investigate your issue, please follow the contributing guidelines.

@MarshallOfSound
Copy link
Member

This is expected behavior, getFileIcon returns the icon for the file type of that file rather than the file itself. E.g. This would work as expected for a photoshop file

@MarshallOfSound
Copy link
Member

GitHub issues are for feature requests and bug reports, questions about using Electron or code assistance requests should be directed to the community or to the Slack Channel.

@bdsexton
Copy link

bdsexton commented Apr 8, 2019

I think this should be reopened.

This is expected behavior…

Not to @oliverschwendener, it seems, and not to me either.

getFileIcon returns the icon for the file type of that file rather than the file itself.

That description of the current behavior explains exactly why the current behavior should not be expected. As acknowledged in the description, a file and a file type are not the same thing. A file is a specific thing, which can and often does have its own icon apart from any file type icon.

As the name of the method in question is getFileIcon rather than getFileTypeIcon I think developers should expect it to return a file-specific icon if available, not a file type icon except perhaps as a fallback (and even then only with some way for developers to discern which is being returned). When item-specific data is available, returning item-class data in response to a method that seems to refer to a specific item seems like either a bug or a design error—a misleading name. You wouldn't expect a method named getUserAvatar to return a generic role image or a generic user image when a user-specific image is available, would you?

For a method named getFileIcon I expect functionality something like this:

  1. [PRIMARY] Get the file-specific icon (e.g., the app icon or an image preview) if available.
  2. [fallback] Get the file-type icon (e.g., an image in PNG format) if available.
  3. [fallback] Get a more general file-type icon (e.g., an image file) if known, available, and preferred according to some system, app, or user setting somewhere. (This one might be flipped with Support window.resizeTo() and window.moveTo() #2 for UX reasons.)
  4. [fallback] Get a generic file icon.

As mentioned above, in the case of #2, #3, and #4 I think there should be some way for developers to tell that fallback icons are being used.

GitHub issues are for feature requests and bug reports…

"app.getFileIcon() does not return the expected file icon" looks like a bug report to me. If that is rejected as a bug report then how about "Please enable app.getFileIcon to get file icons instead of only file type icons" as a feature request?

@sentialx
Copy link
Contributor

This is expected behavior, getFileIcon returns the icon for the file type of that file rather than the file itself. E.g. This would work as expected for a photoshop file

@MarshallOfSound Not true. On Windows, it returns a valid app icon using .exe path, but on macOS not. I think it should be considered as a bug report.

@CyberNika
Copy link

+1

@logidaniel
Copy link

This is still a valid bug on electron v6.0.12, since on Windows I'm getting a real application icon just fine, and on macOS it's just giving back a same generic icon for all the apps.

Looks like this should be a bug for chromium, since electron is just using it's API, but that might never get fixed.

@aguynamedben
Copy link

I would also like this. It's really hard to get app icons otherwise. You have to do a bunch of hacking with NodObjC and ICNS parsing which makes an Electron app prone to native crashes.

@oliverschwendener
Copy link
Author

oliverschwendener commented Feb 5, 2020

@MarshallOfSound This is not a question about using Electron. This is a feature request. Please consider reopening this issue.

In my opinion getFileIcon is a misleading name because it does not return what it promises (see @bdsexton 's reply). I think this function should either return the actual file icon or be renamed to getFileTypeIcon. What do you think?

@giotiskl
Copy link

giotiskl commented May 21, 2020

This is indeed either a bug or an unfortunate name for the method.

@oliverschwendener
Copy link
Author

@MarshallOfSound So I guess this is solved now with app.createThumbnailFromPath() #24802?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants