Bug 412737

Summary: "Save As..." dialog interprets certain user actions as "save in this directory" instead of "descend into this directory"
Product: [Frameworks and Libraries] frameworks-kio Reporter: Szczepan Hołyszewski <rulatir>
Component: Open/save dialogsAssignee: David Faure <faure>
Status: RESOLVED FIXED    
Severity: major CC: a.samirh78, bugs, esmb, kdelibs-bugs, meven29, nate, poprocks
Priority: NOR Keywords: usability
Version: 5.62.0   
Target Milestone: ---   
Platform: Arch Linux   
OS: Linux   
Latest Commit: Version Fixed In: 5.69
Sentry Crash Report:

Description Szczepan Hołyszewski 2019-10-08 18:44:50 UTC
SUMMARY

When a directory is selected and highlighted in the Save As... dialog as a result of incremental search (i.e. typing while the filelist widget has focus) or as a result of selection with arrow keys, pressing the Enter key immediately confirms the dialog and returns the selected directory into the calling application. This behaviour is incorrect and needs to be fixed.


STEPS TO REPRODUCE
1. Trigger the KDE Save As dialog from any application (preferably Firefox for certain reproducibility; must be configured to use KDE file dialogs).
2. Use incremental search or up/down arrows to select a directory entry.
3. Press Enter.

OBSERVED RESULT

The dialog is closed and the path returned to the application is that of the directory that was selected, plus the suggested filename from the filename control.

EXPECTED RESULT

The file selector should descend into the highlighted directory.

SOFTWARE/OS VERSIONS
Operating System: Arch Linux 
KDE Plasma Version: 5.16.5
KDE Frameworks Version: 5.62.0
Qt Version: 5.13.1
Kernel Version: 5.3.4-arch1-1-ARCH
OS Type: 64-bit
Processors: 8 × AMD FX(tm)-8320 Eight-Core Processor
Memory: 15,6 GiB
Comment 1 Szczepan Hołyszewski 2019-10-08 18:46:45 UTC
It seems that grabbing the mouse is necessary in order to descend into a directory. It cannot be done with keyboard only.
Comment 2 Szczepan Hołyszewski 2019-10-08 18:55:26 UTC
IMPORTANT CORRECTION (too bad it is impossible to edit the original post!!!!)

The application I am experiencing the issue in is Chrome not Firefox, but more importantly, Chrome simply uses KDialog and the bug can be reliably reproduced with:

kdialog --getsavefilename $HOME/index.html
Comment 3 Logan Rathbone 2019-10-09 19:33:03 UTC
I can reproduce.

KDE Plasma Version: 5.16.5, KDE Frameworks Version: 5.62.0, Qt Version: 5.13.1

Slackware Current using packages from AlienBob's "ktown" repo (which use little to no patching from upstream).

This only seems to happen if there is a suggested filename. Eg, open PDF in Okular, try to save it to a different location.

If the filename widget is blank, behaviour works as expected. Eg, fire up "kwrite", type a few letters, and hit Save As.
Comment 4 Nate Graham 2019-10-09 19:48:49 UTC
*** Bug 409738 has been marked as a duplicate of this bug. ***
Comment 5 Nate Graham 2019-10-09 19:49:37 UTC
*** Bug 410653 has been marked as a duplicate of this bug. ***
Comment 6 Szczepan Hołyszewski 2019-10-10 09:54:22 UTC
I request changing severity to major. I agree with the reporter of the duplicate #410653 that this is "a major bug for all keyboard users".
Comment 7 Christoph Feck 2019-10-24 12:12:37 UTC
Regression from bea5b681 ?
Comment 8 Ahmad Samir 2019-11-19 12:17:25 UTC
(In reply to Christoph Feck from comment #7)
> Regression from bea5b681 ?

Confirmed, by reverting it locally. The issue is that the condition in the if in KFileWidgetPrivate::_k_slotViewKeyEnterReturnPressed() :

    if (operationMode == KFileWidget::Saving && (ops->mode() & KFile::File) && ops->selectedItems().isEmpty()) {
 
particularly the last bit, ops->selectedItems().isEmpty():
- navigate using keyboard arrows and select a dir
- press Enter to open that dir
- this newly opened dir has no selected items, so the condition evaluates to true, which saves the file and closes the dialogue altogether.
Comment 9 Nate Graham 2019-11-19 21:08:57 UTC
Nice find. Would you be interested in submitting a patch?
Comment 10 Méven Car 2019-11-20 10:29:51 UTC
I am trying to make one.
The issue I have identified, in some cases, is that KFileWidgetPrivate::_k_slotViewKeyEnterReturnPressed is called right after KDirOperator::Private::_k_slotActivated which changed the directory making the kdiroperator selection empty, making _k_slotViewKeyEnterReturnPressed calling _slotOk when it is called.
_k_slotViewKeyEnterReturnPressed should not get called if _k_slotActivated has just changed directory.
Comment 11 Méven Car 2019-12-04 10:21:51 UTC
Git commit 816fc6f9444fe70cdddc67b94e8027fdaab99c20 by Méven Car.
Committed on 04/12/2019 at 10:21.
Pushed by meven into branch 'master'.

[KFileWidget] Avoid calling slotOk right after the url changed

Summary:
When _k_slotViewKeyEnterReturnPressed is called, KDirOperator::_k_slotActivated is called first potentially opening another directory.
And since the directory changed, the kdiroperator selection is empty, causing then kiowidgets-kdirmodeltest to call slotOk and if a filename was present in the filename field, it will cause the dialog to accept() prematurely.

This patch prevents KDirOperator::keyEnterReturnPressed to be emitted when QAbstractItemView::activated would be, preventing the issue in the first place.
FIXED-IN: 5.65

Relates to D19824

Test Plan:
1. Save a file using KFileWidget
2. Go to a folder with files and directories
3. Select a file
4. Select a directory
5. Hit Enter

Before:
The directory is opened briefly and the dialog returns.

After:
The selected directory is opened.

Reviewers: #frameworks, ngraham, elvisangelaccio, dfaure

Reviewed By: ngraham, dfaure

Subscribers: ahmadsamir, feverfew, kde-frameworks-devel

Tags: #frameworks

Differential Revision: https://phabricator.kde.org/D25420

M  +13   -10   src/filewidgets/kdiroperator.cpp
M  +0    -2    src/filewidgets/kdiroperator.h

https://commits.kde.org/kio/816fc6f9444fe70cdddc67b94e8027fdaab99c20
Comment 12 Nate Graham 2020-02-17 00:21:11 UTC
*** Bug 417714 has been marked as a duplicate of this bug. ***
Comment 13 Nate Graham 2020-02-17 00:21:31 UTC
I can still reproduce the original issue. :(
Comment 14 Méven Car 2020-02-17 12:25:03 UTC
Patch proposal: https://phabricator.kde.org/D27455
Comment 15 Logan Rathbone 2020-03-29 02:27:38 UTC
I can still reproduce:

Operating System: Slackware Current
KDE Plasma Version: 5.18.3
KDE Frameworks Version: 5.68.0
Qt Version: 5.13.2
Comment 16 Méven Car 2020-04-04 09:01:38 UTC
Git commit 6327a9f05eb242f0a5b3dd2b5deff370cfae9fa3 by Méven Car.
Committed on 04/04/2020 at 09:01.
Pushed by meven into branch 'master'.

FileWidgets: Ignore Return events from KDirOperator

Summary:
Activated event is used to handle key returns from KDirOperator.
Making a Return in the KDirOperator would cause a double treatment of the event :
 - first KDirOperator::activated for the activated event
 - second KDirOperator::activated for the KeyPressEvent Key_Return
FIXED-IN: 5.69

Test Plan:
 - Open Kate
 - Ctrl+Maj + S, to save file as
 - type a file name
 - go to a directory using the arrows keys and return key

Before:
The dialog returns immediately with the file name in the opened directory.

After:
The directory is opened, the user can continue specifying path.

Reviewers: dfaure, ngraham, #frameworks

Reviewed By: dfaure, ngraham

Subscribers: ahmadsamir, kde-frameworks-devel

Tags: #frameworks

Differential Revision: https://phabricator.kde.org/D27455

M  +7    -0    src/filewidgets/kfilewidget.cpp

https://commits.kde.org/kio/6327a9f05eb242f0a5b3dd2b5deff370cfae9fa3