Skip to content

Fixed #114425 prevent mime pollution on install#209510

Merged
rzhao271 merged 1 commit intomicrosoft:mainfrom
telamon:patch-1
Apr 17, 2024
Merged

Fixed #114425 prevent mime pollution on install#209510
rzhao271 merged 1 commit intomicrosoft:mainfrom
telamon:patch-1

Conversation

@telamon
Copy link
Contributor

@telamon telamon commented Apr 4, 2024

This software is not a plain text editor, and it's definitely not a file manager. Advertising compatibility with such overrides default OS handlers on many distributions, unnecessarily.

This software is not a plain text editor, and it's definitely not a file manager.
Advertising compatibility with such overrides default OS handlers on many distributions, unnecessarily.
@deepak1556
Copy link
Collaborator

Change was originally made for #15741 to list VS Code as an option (not the default) for opening directories and files. Which distribution are you seeing the issue with ? Also did you have a default handler before VS Code was installed xdg-mime query default inode/directory ?

Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

The most recent understanding I have of this is that the distro is misbehaving as we list mime types that we do handle - we certainly do handle plain text and folders.

From the KDE .desktop docs: https://develop.kde.org/docs/features/desktop-file/

The MimeType field describes the MIME types used by your application, meaning it will influence whether your app will show up as an available option to open or run a certain file. For example, a plain text editor would use plain/text to show up when right clicking a text file and selecting "Open With"

@Tyriar Tyriar requested a review from rzhao271 April 4, 2024 19:33
@Tyriar
Copy link
Member

Tyriar commented Apr 4, 2024

Do you have any links to documentation that say the opposite?

@telamon
Copy link
Contributor Author

telamon commented Apr 5, 2024

@Tyriar the distro is not at fault here.

The problem keeps recurring on package upgrade, so for years I've sought guidance at #114425 .
It's a bit tiresome.

@rzhao271
Copy link
Collaborator

rzhao271 commented Apr 5, 2024

An idea I have is to record down the user's default file explorer before updating the desktop and MIME databases, and afterwards, use xdg-mime to set the default for inode/directory back to their preferred file explorer. I'm unsure whether the distro has a default value set in the first place for that MIME type, and whether the xdg-mime command is available.

I haven't seen any command that allows us to update the desktop and MIME databases while allowing us to state that we explicitly don't want to be the default application for a given MIME type.

@telamon
Copy link
Contributor Author

telamon commented Apr 17, 2024

@deepak1556 I'm seeing the behaviour on voidlinux in KDE/Dolphin and Sway/Nemo.
is #15741 a nautilius specific remedy?

I don't think having VSCode advertising inode/directory is the right way to go.
I suspect the priority and/or hijack issue might be as simple as alphabetic order of the executable named code, but I don't know how to verify that.

Here's another report of people affected by this issue:
https://bbs.archlinux.org/viewtopic.php?id=282278

Please help make it stop 🙏

@rzhao271
Copy link
Collaborator

I believe the issue is that the commands we use don't control what order the entry gets placed into the file. I've had similar issues with brave, and so I believe it is an alphabetical order issue. I wrote up a comment at #114425 (comment)

Copy link
Collaborator

@rzhao271 rzhao271 left a comment

Choose a reason for hiding this comment

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

Discussed offline. Merging the PR in to mitigate the issue.

@rzhao271 rzhao271 added this to the April 2024 milestone Apr 17, 2024
@rzhao271 rzhao271 enabled auto-merge (squash) April 17, 2024 19:54
@rzhao271 rzhao271 requested a review from Tyriar April 17, 2024 20:22
@rzhao271 rzhao271 merged commit f1c0071 into microsoft:main Apr 17, 2024
@deepak1556
Copy link
Collaborator

@rzhao271 we should reopen #15741 if the behavior has now been reverted.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants