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

[windows] When creating explorer favorite use more specific windows functions #2719

Closed
remixtj opened this issue Jan 19, 2015 · 11 comments · Fixed by #2920
Closed

[windows] When creating explorer favorite use more specific windows functions #2719

remixtj opened this issue Jan 19, 2015 · 11 comments · Fixed by #2920
Assignees
Labels
ReadyToTest QA, please validate the fix/enhancement sev3-medium type:bug
Milestone

Comments

@remixtj
Copy link
Contributor

remixtj commented Jan 19, 2015

I'm experiencing problems with the creation of the favourite on Windows 7, using folder redirection. I made some research for the problem and found out that client tries to create a link on the wrong place. My "Links" directory is on a cifs share: \server\homes\username\Links. This wrong behavior is caused by the function setupFavLink_private in utility_win.cpp

At the moment in function setupFavLink_private at line 24 the function

    SHGetSpecialFolderPath(0, path, CSIDL_PROFILE, FALSE);

is used.
MSDN documentation says that ShGetFolderPath should be used. But says also that ShGetFolderPath is deprecated. A note states:

Note As of Windows Vista, this function is merely a wrapper for SHGetKnownFolderPath. The CSIDL value is translated to its associated KNOWNFOLDERID and then SHGetKnownFolderPath is called.
New applications should use the known folder system rather than the older CSIDL system, which is supported only for backward compatibility.

Please note that SHGetKnownFolder is a function available only on Vista and later. But, you can also note that also favourites on explorer are available since that windows release.

I made a test and tried to print the path value that is returned by the function call:

    SHGetKnownFolderPath(FOLDERID_Links, 0, NULL, path);

and i get (correctly) the value of \server\homes\username\Links.
I printed also the output of

    SHGetSpecialFolderPath(0, path, CSIDL_PROFILE, FALSE);

and i got _C:\Users\username_, that is correctly my profile home, but the path where the function will create the link is C:\Users\username\Links, that is wrong.


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@remixtj
Copy link
Contributor Author

remixtj commented Mar 11, 2015

@danimo what do you think about this new patch? better?

@jturcotte
Copy link
Member

Reopening since we had to revert this in #3562

@jturcotte jturcotte reopened this Aug 6, 2015
@hodyroff
Copy link

Is this current in master? What exactly is the issue from a UX perspective?

@remixtj
Copy link
Contributor Author

remixtj commented Dec 12, 2016

Hi,
this has been reverted. I had no possibility to have contributor terms signed, but since my company is a customer of owncloud, this patch is for sure interesting.

The UX impact is that the user on windows, if using a Group Policy that sets its Links directory to a path different to %USERPROFILE%, the link to ownCloud directory on Explorer's Favourites is not available.

@guruz
Copy link
Contributor

guruz commented Mar 29, 2017

@remixtj Any news from your side?

@remixtj
Copy link
Contributor Author

remixtj commented Mar 29, 2017

@guruz got a mail 3 months ago from @hodyroff and i replied saying that i can sign personally CLA if is not an issue from owncloud side since is a my own contribution, even if done during worktime.

If is ok, i can sign and then submit the patch again.

@hodyroff
Copy link

Works, thank you!

@remixtj
Copy link
Contributor Author

remixtj commented Apr 10, 2017

@guruz PR is now available, with signed CLA

@guruz guruz removed the Needs info label Apr 24, 2017
@guruz guruz added this to the 2.4.0 milestone Apr 26, 2017
@jturcotte jturcotte reopened this Apr 27, 2017
@jturcotte jturcotte added the ReadyToTest QA, please validate the fix/enhancement label Apr 27, 2017
@SamuAlfageme
Copy link
Contributor

Just tested current master and ownCloud shorcut is back in the Favorites 🎉 looks like we can cross one of #455 (comment) items.

@remixtj would there be a way to set the folder icon to owncloud.exe's?. I've read about Desktop.ini files but, doesn't seem elegant enough:

[.ShellClassInfo]
IconFile=%ProgramFiles%\ownCloud\owncloud.exe
IconIndex=0

I'll repeat the test on W8.1 & 10 and close here if everything continues smooth there!

@remixtj
Copy link
Contributor Author

remixtj commented May 4, 2017

Hello,

as far as i know, through QT is not possible to set the link icon, since that qt functions is cross platform and makes symlinks in linux and .lnk files in Windows.

I'll take a look if some other options are available

@SamuAlfageme
Copy link
Contributor

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ReadyToTest QA, please validate the fix/enhancement sev3-medium type:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants