Bug Hunting Session
Bug 105538 - Download destination folder should be default to Download folder on Windows
Summary: Download destination folder should be default to Download folder on Windows
Status: RESOLVED WORKSFORME
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
5.3.0.2 rc
Hardware: All Windows (All)
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:6.0.0
Keywords: needsDevAdvice
Depends on:
Blocks: a11y-Windows
  Show dependency treegraph
 
Reported: 2017-01-26 03:21 UTC by Volga
Modified: 2017-12-22 15:29 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
Screenshot (28.17 KB, image/png)
2017-01-26 03:22 UTC, Volga
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Volga 2017-01-26 03:21:29 UTC
Description:
Currently the download destination is default to Desktop folder on Windows, but this folder is intended to save quick launches used for Windows Desktop, no intended to save downloaded files.

Steps to Reproduce:
1. Tools -> Options -> LibreOffice -> Online Update

Actual Results:  
The download destination is default to C:\Users\<User name>\Desktop, as the attachment.

Expected Results:
Despite it can be replaced via Change button, download destination should defaulting to C:\Users\<User name>\Download folder.


Reproducible: Always

User Profile Reset: No

Additional Info:


User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:51.0) Gecko/20100101 Firefox/51.0
Comment 1 Volga 2017-01-26 03:22:17 UTC
Created attachment 130688 [details]
Screenshot
Comment 2 V Stuart Foote 2017-01-26 14:19:08 UTC
Somewhat a legacy setting pre Vista, intent being to direct the download to a directory user has write permission to and will find. Or, failing that to a common location with write permissions for all users [1][2].

Probably reasonable to make use of the now Windows OS default provision of a writable %USERPROFILE%/Downloads directory.

The getDesktopDirectory() seems to only be used for the online update, so should be able to adjust it perhaps to getDownloadsDirectory()

Easy hack?

=-ref-=

[1] http://opengrok.libreoffice.org/xref/core/extensions/source/update/check/updatecheckconfig.hxx#140

[2] http://opengrok.libreoffice.org/xref/core/extensions/source/update/check/updatecheckconfig.cxx#163
Comment 3 V Stuart Foote 2017-01-26 15:11:02 UTC
(In reply to V Stuart Foote from comment #2)
> Probably reasonable to make use of the now Windows OS default provision of a
> writable %USERPROFILE%/Downloads directory.
> 

Was no CLSID (constant special item ID list) defined for Downloads with old system, why Desktop is now the default for our downloads. 

For the new its KNOWNFOLDERID handle is "FOLDERID_Downloads" accessed with the SHGetKnownFolderPath() function. Still looks pretty straight forward substitution.

=-refs-=

https://msdn.microsoft.com/en-us/library/windows/desktop/dd378457%28v=vs.85%29.aspx

https://msdn.microsoft.com/en-us/library/windows/desktop/bb762188%28v=vs.85%29.aspx

http://stackoverflow.com/questions/28963848/shgetfolderpath-download-folder
Comment 4 Heiko Tietze 2017-01-26 19:55:23 UTC
(In reply to V Stuart Foote from comment #2)
> ... %USERPROFILE%/Downloads... Easy hack?

Agreed (-needsUX) and likely (+easyhack).
Comment 5 Samuel Mehrbrodt (CIB) 2017-01-27 09:07:31 UTC
easyHack and needsDevEval are mutually exclusive. Removed easyHack for now.
Comment 6 jani 2017-01-27 09:17:54 UTC
(In reply to Samuel Mehrbrodt (CIB) from comment #5)
> easyHack and needsDevEval are mutually exclusive. Removed easyHack for now.

???? we use that combination for more than 1 year.

If an easyhack missed e.g. code pointers needsDevEval is set. Please do a search, to see how often that combination is used.

Did you think of needsDevAdvice, which is used for developer advice (whether or not a think is doable at all).
Comment 7 V Stuart Foote 2017-01-27 13:56:01 UTC
Another facet, the change from CLSID to KNOWNFOLDERID based method is only supported from MS Windows Vista onward. We will need to consider provision for update of Windows XP while that remains a "supported" OS.
Comment 8 jani 2017-01-27 14:22:21 UTC
removing easyhack, it does not look like something a new contrbutor can "just" do.
Comment 9 Volga 2017-04-07 07:53:40 UTC Comment hidden (obsolete)
Comment 10 Volga 2017-06-28 13:16:09 UTC
(In reply to V Stuart Foote from comment #7)
> Another facet, the change from CLSID to KNOWNFOLDERID based method is only
> supported from MS Windows Vista onward. We will need to consider provision
> for update of Windows XP while that remains a "supported" OS.
LibreOffice removed support for Windows XP and Vista, so we may not necessary to consider them.
Comment 11 Takeshi Abe 2017-08-02 23:00:45 UTC
By the way, is it better or not to apply a similar change to other OS than Windows too?
Comment 12 V Stuart Foote 2017-08-03 00:32:38 UTC
(In reply to Takeshi Abe from comment #11)
> By the way, is it better or not to apply a similar change to other OS than
> Windows too?

@moggi, this covers Windows but is controling a suitable download location for updates going to need attention cross platform moving forward with your MAR based incrementals?
Comment 13 Markus Mohrhard 2017-08-03 00:50:05 UTC
(In reply to V Stuart Foote from comment #12)
> (In reply to Takeshi Abe from comment #11)
> > By the way, is it better or not to apply a similar change to other OS than
> > Windows too?
> 
> @moggi, this covers Windows but is controling a suitable download location
> for updates going to need attention cross platform moving forward with your
> MAR based incrementals?

First this bug report is about the old update checker and not the new automatic updater. Most likely this bug will go away at some point just by removing the old update checker.

The new updater is going to store the update files in the LibreOffice user profile for now.
Comment 14 Volga 2017-08-03 13:36:46 UTC
(In reply to Markus Mohrhard from comment #13)
> (In reply to V Stuart Foote from comment #12)
> > (In reply to Takeshi Abe from comment #11)
> > > By the way, is it better or not to apply a similar change to other OS than
> > > Windows too?
> > 
> > @moggi, this covers Windows but is controling a suitable download location
> > for updates going to need attention cross platform moving forward with your
> > MAR based incrementals?
> 
> First this bug report is about the old update checker and not the new
> automatic updater. Most likely this bug will go away at some point just by
> removing the old update checker.
> 
> The new updater is going to store the update files in the LibreOffice user
> profile for now.

Oh yeah, that’s a good choice.
Comment 15 Commit Notification 2017-08-18 10:58:56 UTC
Takeshi Abe committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=fb0bf56948d7f968717f4a3f53ce56e320cd5f36

tdf#105538 Download update to Downloads folder on Windows

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 16 Xisco Faulí 2017-09-18 07:48:31 UTC Comment hidden (obsolete)
Comment 17 Xisco Faulí 2017-11-05 16:24:41 UTC
A polite ping to Takeshi Abe: is this bug fixed? if so, could you please close it as RESOLVED FIXED ? Thanks
Comment 18 Volga 2017-12-22 15:09:48 UTC
OK, this is fixed.

Version: 6.0.0.1 (x64)
Build ID:d2bec56d7865f05a1003dc88449f2b0fdd85309a
CPU 线程:4; 操作系统:Windows 10.0; UI 渲染:默认; 
区域语言:zh-CN (zh_CN); Calc: group