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

ShellExt: Move file extensions list from XML to DLL + implement .lnk shortcut handling #1006

Merged
merged 2 commits into from
Jan 12, 2025

Conversation

cjee21
Copy link
Collaborator

@cjee21 cjee21 commented Jan 3, 2025

See #994 (comment) and #998 (comment)

Known limitations:
If shortcut target is a long path, it does not work due to path length limitations of IShellLink::GetPath.

@cjee21
Copy link
Collaborator Author

cjee21 commented Jan 3, 2025

I give up on implementing shortcut handling. Seems too much work.

After this PR, MediaInfo will likely not appear in classic context menu for shortcuts since we are now doing extension checking ourselves. Before, Windows will parse shortcuts and check if destination extension is in our manifest when using the classic menu.
Turns out it still works as before so no regression for this case.

@cjee21
Copy link
Collaborator Author

cjee21 commented Jan 3, 2025

Updated file extensions lists generator: MediaInfo_fileextensions.xlsx

@cjee21
Copy link
Collaborator Author

cjee21 commented Jan 3, 2025

MediaInfo entry is showing on archives now.....

@cjee21
Copy link
Collaborator Author

cjee21 commented Jan 3, 2025

MediaInfo entry is showing on archives now.....

Fixed in force push.

Also noticed MediaInfo is now no longer the first entry in classic context menu. Hope it won't undo the effects of the previous Verb Id renaming workarounds.

@cjee21
Copy link
Collaborator Author

cjee21 commented Jan 4, 2025

I give up on implementing shortcut handling. Seems too much work.

Another attempt. Not easy but managed to get it working. :D

Now need to rebase either this or #1002 else there will be no shortcut handling for multi-instance.

Also drag and drop in MediaInfo does not handle shortcuts as well as opening a folder containing shortcuts.

For .url files, I think better to handle it in GUI itself along with drag&drop.

@cjee21
Copy link
Collaborator Author

cjee21 commented Jan 4, 2025

@JeromeMartinez Ubuntu-latest runner on GitHub has updated to 24.04, 'libwxgtk3.0-gtk3-dev' no longer exists causing CI failure.

Update: Fixed in #1007

@cjee21 cjee21 changed the title ShellExt: Move file extensions list from XML to DLL ShellExt: Move file extensions list from XML to DLL + implement .lnk shortcut handling Jan 4, 2025
@cjee21 cjee21 force-pushed the shell-exts branch 7 times, most recently from a0beaac to cd92d03 Compare January 7, 2025 16:59
@JeromeMartinez
Copy link
Member

Snapshot.

@cjee21
Copy link
Collaborator Author

cjee21 commented Jan 12, 2025

Snapshot.

Appears to work fine on my PC.

The force push now is just a rebase on current master.

@JeromeMartinez JeromeMartinez merged commit 7b2ba0b into MediaArea:master Jan 12, 2025
3 checks passed
@cjee21 cjee21 deleted the shell-exts branch January 12, 2025 14:02
@JeromeMartinez
Copy link
Member

Another attempt. Not easy but managed to get it working. :D

Great, good to use all the possibilities with a DLL for shell extension.
Trying to understand, from which version does it work? I see that it works on my Win10.

Now need to rebase either this or #1002 else there will be no shortcut handling for multi-instance.

I see that you rebased/updated while I was writing, thank you again!

Also drag and drop in MediaInfo does not handle shortcuts as well as opening a folder containing shortcuts.

Not sure I understand well, with my test drag and drop does not work for .lnk for both directly the file or directory having it.
Anyway, it is weird that the shell extension supports it but not the library, I need to port your code to the library directly.

For .url files, I think better to handle it in GUI itself along with drag&drop.

In my opinion it should be in both the shell extension and the library, like .lnk, having it in the GUI (actually the library) would not permit the same behavior as with .lnk. But it is something I can start in the library (handling corner cases e.g. '?' in HTTPS URLs) then you copy my code (or I do it, I feel confortable enough now about it for doing it too, I am just slower than you in this area).

@cjee21
Copy link
Collaborator Author

cjee21 commented Jan 12, 2025

Great, good to use all the possibilities with a DLL for shell extension. Trying to understand, from which version does it work? I see that it works on my Win10.

Only for new Windows 11 context menu. For classic context menu on both Windows 11 and 10, Windows handles shortcuts as previously mentioned. Also as noted on first post, when shortcut leads to a long path, it may not work due to limitations of the API used. Not sure how Windows and Notepad shell extension handles shortcuts pointing to long paths, maybe they use different API but don't know what. Since long paths are rare and other parts of MediaInfo (legacy shell, drag&drop, open dialog) do not support it too, I do not think it is a big problem. Rather, it is a bonus that the new shell extension can open long paths in some cases.

Not sure I understand well, with my test drag and drop does not work for .lnk for both directly the file or directory having it. Anyway, it is weird that the shell extension supports it but not the library, I need to port your code to the library directly.

Yes, it does not work. When opening a directory containing shortcuts from shell extension, it does not work too since shell will only pass the directory to MediaInfo and the rest is up to MediaInfoLib in this case.

In my opinion it should be in both the shell extension and the library, like .lnk, having it in the GUI (actually the library) would not permit the same behavior as with .lnk. But it is something I can start in the library (handling corner cases e.g. '?' in HTTPS URLs) then you copy my code (or I do it, I feel confortable enough now about it for doing it too, I am just slower than you in this area).

My opinion is the shell extension will just pass all .url files to MediaInfo since these files usually contain web urls. The urls may not have file extensions and even if they did, it is probably not a good idea to make a network connection from the shell extension to check for the extension.

@JeromeMartinez
Copy link
Member

For classic context menu on both Windows 11 and 10, Windows handles shortcuts as previously mentioned

I actually was not aware that Win10 was doing itself the resolution of the name 😅.
Sad that it does not do it with the shell extension DLL (passing the real file name), great that you were able to do it.

My opinion is the shell extension will just pass all .url files to MediaInfo since these files usually contain web urls. The urls may not have file extensions and even if they did, it is probably not a good idea to make a network connection from the shell extension to check for the extension.

I am more into doing like with .lnk, resolving the name and checking the extension of the resolved name (not doing the network connection), in both the shell extension and the library.

@JeromeMartinez
Copy link
Member

Since long paths are rare and other parts of MediaInfo (legacy shell, drag&drop, open dialog)

Nobody complains yet but it may be something good to fix at some point, anyway far from important.

@cjee21
Copy link
Collaborator Author

cjee21 commented Jan 12, 2025

I am more into doing like with .lnk, resolving the name and checking the extension of the resolved name (not doing the network connection), in both the shell extension and the library.

Oh ya, no need network connection now that I think about it. Just need read the correct line from the file. But there will be issue when the url leads to a video file but does not have a video file extension (not idea how common is this). Also need special handling for ? as you mentioned.


There's also the issue that Qt one currently does not handle urls as command parameters or as input to file open dialog so need some changes there as well.

@JeromeMartinez
Copy link
Member

But there will be issue when the url leads to a video file but does not have a video file extension

Similar to .lnk: no MediaInfo line.

There's also the issue that Qt one currently does not handle urls as command parameters or as input to file open dialog so need some changes there as well.

I plan to add that to the MediaInfo library so it would be independent from the UI.

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

Successfully merging this pull request may close these issues.

2 participants