Bug 109537 - Files coming from Temporary Internet Files temp dir are offered to be saved back in that directory instead of default workdir
Summary: Files coming from Temporary Internet Files temp dir are offered to be saved b...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: framework (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All Windows (All)
: medium normal
Assignee: Mike Kaganski
URL:
Whiteboard: target:6.0.0
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-28 04:11 UTC by Aron Budea
Modified: 2017-08-02 00:35 UTC (History)
0 users

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Aron Budea 2017-07-28 04:11:58 UTC
As far as I know this issue only concerns Windows and not Linux, no idea about OSX.

- Open a file directly from browser in LO (without saving it first), let's say attachment 134280 [details]. File is downloaded in a temporary directory, and opens in read-only mode.
- Click Save As...

=> File is offered to be saved in the temporary download directory (eg. C:\Users\<User>\AppData\Local\Temp or C:\Users\<user>\AppData\Local\Microsoft\Windows\Temporary Internet Files\...) instead of the default folder for documents.


LO is supposed to be prepared to not suggest the system temporary directory:
https://opengrok.libreoffice.org/xref/core/sfx2/source/doc/guisaveas.cxx#1151
// Suggest somewhere other than the system's temp directory
if( aLocation.GetMainURL( INetURLObject::DecodeMechanism::NONE ).startsWith( SvtPathOptions().GetTempPath() ) )

Note that the Windows file picker saves history, and LO generally takes that into account and doesn't force its own suggestion in that case (see https://opengrok.libreoffice.org/xref/core/fpicker/source/win32/filepicker/VistaFilePickerImpl.cxx#688 ).

Nevertheless, the mechanism doesn't work, and the temporary directory is often suggested in such case.
Apart from it not working, there are the following problems with this approach:
- There's not one single temporary directory.
- The mechanism looks for LO's Temp path config for temporary directory. By default this is the system temp dir, but can be changed, and then it's not the same as the system temp dir anymore.

LO's default temp path (eg. $(temp)) is substituted using osl::FileBase::getTempDirURL(...).

Observed using 6.0 master build (a4bab9609b04cb644859cf548bb4739a9d5aa590) & 3.3.0 / Windows 7.
Comment 1 Aron Budea 2017-07-28 04:18:50 UTC
A solution attempt could be having a dedicated function to check if a given location is inside of any system temporary directory (those that aren't the default temp dir have to be found out in system-specific ways).

Then it also has to be investigated why LO doesn't already suggest My Documents if the file is in the currently identified temp dir.
Comment 2 Aron Budea 2017-07-31 11:51:01 UTC
The plan is to update the followig piece of code (added for bug 80807):
https://opengrok.libreoffice.org/xref/core/sfx2/source/doc/guisaveas.cxx#1151
// Suggest somewhere other than the system's temp directory
if( aLocation.GetMainURL( INetURLObject::DecodeMechanism::NONE ).startsWith( SvtPathOptions().GetTempPath() ) )

1. Use osl::FileBase::getTempDirURL(...) instead of SvtPathOptions().GetTempPath() to get system temporary directory, and check against that.

2. Add an additional, Windows-specific check, get the Temporary Internet Files location, and check if path is inside that.

a. Before Vista (and kept for compatibility reasons):

Call to SHGetFolderPath can be used:
https://msdn.microsoft.com/en-us/library/windows/desktop/bb762181(v=vs.85).aspx

The value for nFolder is CSIDL_INTERNET_CACHE (from here:
https://msdn.microsoft.com/en-us/library/windows/desktop/bb762494(v=vs.85).aspx )

b. Since Vista:

Call to SHGetKnownFolderPath can be used:
https://msdn.microsoft.com/en-us/library/windows/desktop/bb762188(v=vs.85).aspx

The value for rfid is FOLDERID_InternetCache (from here:
https://msdn.microsoft.com/en-us/library/windows/desktop/bb762494(v=vs.85).aspx )
Comment 3 Mike Kaganski 2017-07-31 14:07:26 UTC
A patch is in gerrit: https://gerrit.libreoffice.org/40594
Comment 4 Aron Budea 2017-07-31 14:56:12 UTC
An addition to my original description:

Since 5.4 the default temporary directory isn't concerned anymore, as Justin's fix to bug 80807 handles that, I was just testing in 5.3.4 first, which didn't have it. A small additional change is that we don't want to check against LO's temporary directory there, as it might be changed by the user, and thus not be the same as the system's.

For repro with 5.4 use Internet Explorer / Outlook, as those don't use the default temporary directory in Windows, but Temporary Internet Files. The fix is for handling that.
Comment 5 Commit Notification 2017-07-31 17:58:36 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

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

tdf#109537: treat Temporary Internet Files as temp directory

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 6 Aron Budea 2017-08-02 00:35:11 UTC
Fixed now, thanks Mike!