Bug 459485

Summary:	Recently used files cannot be chosen in file pickers
Product:	[Frameworks and Libraries] frameworks-kio	Reporter:	ph
Component:	Open/save dialogs	Assignee:	KIO Bugs <kio-bugs-null>
Status:	RESOLVED FIXED
Severity:	normal	CC:	bugseforuns, kdelibs-bugs, meven29, nate, sephiroth_pk
Priority:	VHI	Keywords:	regression
Version:	5.98.0
Target Milestone:	---
Platform:	Other
OS:	Linux
Latest Commit:	https://invent.kde.org/frameworks/kio/-/commit/2386f3304c8ea0f2cda0baa0d31ee1f9248951d1	Version Fixed In:	5.109
Sentry Crash Report:
Attachments:	Screenshot of the error

Description ph 2022-09-21 10:27:03 UTC

SUMMARY
***
When choosing a file from the "Recent files" pane in both Firefox and Chrome an error occurs with a message like:
The file "recentlyused:/files/Screenshot_20220921_112610.png-0" could not be found

The filename seems to include an extra "-0" at the end. Removing that inside the file picker doesn't help.

This can be tested for instance here: https://ps.uci.edu/~franklin/doc/file_upload.html
***


STEPS TO REPRODUCE
1. Go to https://ps.uci.edu/~franklin/doc/file_upload.html
2. Try to upload any file through the "Recent files"
3. An error message appears. The filename includes "-<number>" in the end.

OBSERVED RESULT
Error message

EXPECTED RESULT
File is uploaded

SOFTWARE/OS VERSIONS
Operating System: Arch Linux
KDE Plasma Version: 5.25.5
KDE Frameworks Version: 5.98.0
Qt Version: 5.15.6
Kernel Version: 5.19.9-arch1-1 (64-bit)
Graphics Platform: X11

ADDITIONAL INFORMATION
In order to test this in Firefox you need to go to about:config and set `widget.use-xdg-desktop-portal.file-picker` to `1`.

Comment 1 Nate Graham 2022-09-22 19:56:08 UTC

Are you talking about the KDE file picker or the GTK one? Can you attach a screenshot of it?

Comment 2 ph 2022-09-23 08:22:04 UTC

Created attachment 152360 [details]
Screenshot of the error

Comment 3 ph 2022-09-23 08:22:52 UTC

(In reply to Nate Graham from comment #1)
> Are you talking about the KDE file picker or the GTK one? Can you attach a
> screenshot of it?

I think I'm talking about the KDE file picker. Firefox showed me the GTK one until I changed FF settings as described in the OP.

Screenshot added!

Comment 4 Nate Graham 2022-09-23 15:07:38 UTC

Thanks, looks like the KDE one. Can reproduce the issue.

Comment 5 Nate Graham 2022-09-23 15:07:47 UTC

*** Bug 459563 has been marked as a duplicate of this bug. ***

Comment 6 Méven Car 2022-09-24 20:45:19 UTC

Is the bug only for portal dialogs ?
The screenshot shows the portal File picker is used.

Comment 7 Riccardo Robecchi 2022-09-24 20:46:33 UTC

(In reply to Méven Car from comment #6)
> Is the bug only for portal dialogs ?
> The screenshot shows the portal File picker is used.

No, the behaviour is the same when using e.g. Kate.

Comment 8 Méven Car 2022-09-25 07:42:17 UTC

Notes for bug resolver:

The bug is due to the fact that recentlyused:/ can contain several time a file with the same filename. But those files still need to be identified individually. Furthermore the order is determined by the underlying query to kactivitiesStats.
We have a mechanism in KIO to hide the technical name to the user, when we fill UDS_DISPLAY_NAME, the name is only revealed when displaying its real url or path. Currently recentlyused:/ code is exploiting this but this can't fulfill the need here. To get to identify files individually (this is also a requirement of using KDirOperator/KDirModel), -n is appended to the url where n is the number of the row is added to the technical name. This allows recentlyused to be able to decode and identify files with urls such `recentlyused:/files/CMakeLists.txt-10` .

This mostly works in dolphin. But FileWidget (KIO File picker) needs to return urls or path to its user, so it bluntly return the technical url. 
recentlyused:/ fills UDS_LOCAL_PATH and UDS_TARGET_URL, but we lack a way to make FileWidget use it instead (this is done is `KDirOperator::selectDir` for instance). FileWidgets also expects those urls to be filled in the selecturl bar correctly and can cause some other incompatibility issues.

This behavior (hence) this regression was added in https://invent.kde.org/network/kio-extras/-/merge_requests/158/diffs.

Comment 9 Riccardo Robecchi 2023-06-16 13:14:09 UTC

(In reply to Méven Car from comment #8)
> Notes for bug resolver:
> 
> The bug is due to the fact that recentlyused:/ can contain several time a
> file with the same filename. But those files still need to be identified
> individually. Furthermore the order is determined by the underlying query to
> kactivitiesStats.
> We have a mechanism in KIO to hide the technical name to the user, when we
> fill UDS_DISPLAY_NAME, the name is only revealed when displaying its real
> url or path. Currently recentlyused:/ code is exploiting this but this can't
> fulfill the need here. To get to identify files individually (this is also a
> requirement of using KDirOperator/KDirModel), -n is appended to the url
> where n is the number of the row is added to the technical name. This allows
> recentlyused to be able to decode and identify files with urls such
> `recentlyused:/files/CMakeLists.txt-10` .
> 
> This mostly works in dolphin. But FileWidget (KIO File picker) needs to
> return urls or path to its user, so it bluntly return the technical url. 
> recentlyused:/ fills UDS_LOCAL_PATH and UDS_TARGET_URL, but we lack a way to
> make FileWidget use it instead (this is done is `KDirOperator::selectDir`
> for instance). FileWidgets also expects those urls to be filled in the
> selecturl bar correctly and can cause some other incompatibility issues.
> 
> This behavior (hence) this regression was added in
> https://invent.kde.org/network/kio-extras/-/merge_requests/158/diffs.

Considering the "recent files" functionality is quite essential and we're still without it almost one year later, wouldn't it be advisable to revert the linked merge request to restore the "recent files" functionality? I would argue that having the ability to forget recent files is not useful if we cannot use the main functionality at all.

Comment 10 Méven Car 2023-06-17 09:10:32 UTC

(In reply to Riccardo Robecchi from comment #9)
> (In reply to Méven Car from comment #8)
> > Notes for bug resolver:
> > 
> > The bug is due to the fact that recentlyused:/ can contain several time a
> > file with the same filename. But those files still need to be identified
> > individually. Furthermore the order is determined by the underlying query to
> > kactivitiesStats.
> > We have a mechanism in KIO to hide the technical name to the user, when we
> > fill UDS_DISPLAY_NAME, the name is only revealed when displaying its real
> > url or path. Currently recentlyused:/ code is exploiting this but this can't
> > fulfill the need here. To get to identify files individually (this is also a
> > requirement of using KDirOperator/KDirModel), -n is appended to the url
> > where n is the number of the row is added to the technical name. This allows
> > recentlyused to be able to decode and identify files with urls such
> > `recentlyused:/files/CMakeLists.txt-10` .
> > 
> > This mostly works in dolphin. But FileWidget (KIO File picker) needs to
> > return urls or path to its user, so it bluntly return the technical url. 
> > recentlyused:/ fills UDS_LOCAL_PATH and UDS_TARGET_URL, but we lack a way to
> > make FileWidget use it instead (this is done is `KDirOperator::selectDir`
> > for instance). FileWidgets also expects those urls to be filled in the
> > selecturl bar correctly and can cause some other incompatibility issues.
> > 
> > This behavior (hence) this regression was added in
> > https://invent.kde.org/network/kio-extras/-/merge_requests/158/diffs.
> 
> Considering the "recent files" functionality is quite essential and we're
> still without it almost one year later, wouldn't it be advisable to revert
> the linked merge request to restore the "recent files" functionality? I
> would argue that having the ability to forget recent files is not useful if
> we cannot use the main functionality at all.

I have spent some time trying to fix this, it can be fixed. I will get at it again.

Comment 11 Bug Janitor Service 2023-06-22 09:41:27 UTC

A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kio/-/merge_requests/1331

Comment 12 Riccardo Robecchi 2023-06-22 09:43:35 UTC

(In reply to Méven Car from comment #10)
> I have spent some time trying to fix this, it can be fixed. I will get at it
> again.
Thank you for your work, Méven, it is always deeply appreciated.

Comment 13 Méven 2023-06-29 08:40:26 UTC

Git commit ca5e0796f84c37efe69f323f0072a601af34abbf by Méven Car.
Committed on 29/06/2023 at 07:24.
Pushed by meven into branch 'master'.

KFileWidget: Use targetUrl to extract urls, better handle absolute urls cases

M  +34   -2    autotests/kfilewidgettest.cpp
M  +23   -16   src/filewidgets/kfilewidget.cpp
M  +17   -0    tests/kfilewidgettest_gui.cpp

https://invent.kde.org/frameworks/kio/-/commit/ca5e0796f84c37efe69f323f0072a601af34abbf

Comment 14 Méven Car 2023-07-01 09:50:42 UTC

(In reply to Méven from comment #13)
> Git commit ca5e0796f84c37efe69f323f0072a601af34abbf by Méven Car.
> Committed on 29/06/2023 at 07:24.
> Pushed by meven into branch 'master'.
> 
> KFileWidget: Use targetUrl to extract urls, better handle absolute urls cases
> 
> M  +34   -2    autotests/kfilewidgettest.cpp
> M  +23   -16   src/filewidgets/kfilewidget.cpp
> M  +17   -0    tests/kfilewidgettest_gui.cpp
> 
> https://invent.kde.org/frameworks/kio/-/commit/
> ca5e0796f84c37efe69f323f0072a601af34abbf

I have a backport to KF5 https://invent.kde.org/frameworks/kio/-/merge_requests/1339

Comment 15 Méven Car 2023-07-07 07:58:19 UTC

Git commit 2386f3304c8ea0f2cda0baa0d31ee1f9248951d1 by Méven Car, on behalf of Méven Car.
Committed on 07/07/2023 at 07:15.
Pushed by meven into branch 'kf5'.

KFileWidget: Use targetUrl to extract urls, better handle absolute urls cases
(cherry picked from commit ca5e0796f84c37efe69f323f0072a601af34abbf)

M  +34   -2    autotests/kfilewidgettest.cpp
M  +19   -13   src/filewidgets/kfilewidget.cpp
M  +18   -0    tests/kfilewidgettest_gui.cpp

https://invent.kde.org/frameworks/kio/-/commit/2386f3304c8ea0f2cda0baa0d31ee1f9248951d1

Comment 16 Patrick Silva 2023-07-09 15:33:32 UTC

not fixed in frameworks 5.108.

Operating System: Arch Linux 
KDE Plasma Version: 5.27.6
KDE Frameworks Version: 5.108.0
Qt Version: 5.15.10
Graphics Platform: Wayland

Comment 17 Méven Car 2023-07-11 12:27:28 UTC

(In reply to Patrick Silva from comment #16)
> not fixed in frameworks 5.108.
> 
> Operating System: Arch Linux 
> KDE Plasma Version: 5.27.6
> KDE Frameworks Version: 5.108.0
> Qt Version: 5.15.10
> Graphics Platform: Wayland

The fix did not make it to 5.108, it will be in 5.109.