Version: (using KDE 4.2.3) Installed from: Ubuntu Packages When selecting "Move to Trash" from Dolphin's context menu, if the Shift key is depressed then the action performed is "Delete", not "Move to Trash". This should be made clear, preferably by actually changing the text of the context menu message when the Delete key is depressed. It appears that there is in fact facility for detecting keyboard status (including Shift and other modifier keys): http://aseigo.blogspot.com/2009/04/more-on-keyboard-status.html This issue is important because users may either not know that their Shift key is depressed (Sticky Keys users, for example) or may not know that Shift modifies the "Move to Trash" behaviour. This certainly is a dataloss issue as Dolphin permanently deletes files while implying that the files may be recoverable in the Trash.
The code seems to be : dolphinviewactionhandler.cpp:280 void DolphinViewActionHandler::slotTrashActivated(Qt::MouseButtons, Qt::KeyboardModifiers modifiers) { emit actionBeingHandled(); // Note: kde3's konq_mainwindow.cpp used to check // reason == KAction::PopupMenuActivation && ... // but this isn't supported anymore if (modifiers & Qt::ShiftModifier) m_currentView->deleteSelectedItems(); else m_currentView->trashSelectedItems(); } @Peter: could the mousebuttons variable be checked to ensure the shift behavior is not enforced when selecting the "Move to Trash" action in a menu? (if the action was "clicked", some of the mouse buttons flags should be enabled) I wonder if that way to check the behavior could work. Regards
I agree with Dotan that the current behavior is not the expected one. @Dario: Thanks for the pointer, I tried to check the mouse button but it does not work. The action is triggered after the mouse button has been released and it seems that the current state is passed in the parameter. As this signature of triggered() is a KAction- and not a QAction-thing, the chances are good that we can fix this independent from Qt. However currently I've some other priorities to get Dolphin in shape for KDE 4.4 and this is more a corner issue that seems to be more timeconsuming to fix than it looks like in the first sight :-/
(In reply to comment #2) > I agree with Dotan that the current behavior is not the expected one. I could fix that, just tell me what behavior is expected: * to move to trash, regardless of Shift being pressed or not, or * to change "Move to Trash" action to "Delete" action in popup menu whilst Shift key is pressed (this is harder, but I could try to tackle this).
> I could fix that, just tell me what behavior > is expected: I would expect the text of the context menu to change. Otherwise, we would loose the useful feature of shift-delete for bypassing the Trash. Thanks!
Hmm, what should happen if the user pressed SHIFT when opening the context menu and he releases the shift key before selecting the Trash? Instead of changing the text IMO it would be more straight forward from an implementation point of view (and I also think from a users point of view) just to ignore the shift key in the case if the action has been selected by the context menu. IMO the shift-key is just a shortcut for keyboard users. People that want to differ between moving to trash + deleting in the context menu can add the "Delete" action already. So to summarise: - don't change any text - ignore the shift key when the "Move to Trash" action has been selected by the context menu
I investigated a bit on this issue and it seems to me that there are two possible ways to trigger the "move_to_trash" action: - select it in the context menu; the action has be enabled and visible; - press Del (if there are any modifiers pressed, the action isn't triggered; when I deleted shortcut for the "delete" action, Shift+Del didn't trigger any action at all); the action has be enabled. So, if I'm right, a simple conclusion is that there could be any keyboard modifiers present only if the action was selected from the context menu, so the proper resolution is not to check modifiers' state in DolphinViewActionHandler::slotTrashActivated() at all. I tried modifying the code and it behaves as expected.
Created attachment 41596 [details] a patch fixing this, hopefully
Thanks Bartosz for the patch! After seeing your patch it got clear to me, that the current behavior has been added on purpose by David Faure (checked with 'svn blame') - I've added David to CC... @David: Short summary of the discussion: The bug report considers it as dangerous, that pressing the Shift-key while selecting the "Move to Trash" action from the context menu triggers actually the "Delete" action. I initially agreed, however as this has been done on purpose, removing it might harm users that are familiar with the current behavior. It would be great if you could tell us your point of view about whether the code should stay as it is or if you think the "fix" is OK. Thanks.
That is why in comment #4 I mentioned that the text should change. The ability to shift-delete is important, just the text is wrong.
(In reply to comment #9) > That is why in comment #4 I mentioned that the text should change. The ability > to shift-delete is important, just the text is wrong. Well, you wouldn't loose that ability: Shift+Del keyboard shortcut would be still working, and you'd be still able to enable the option to show the "Delete" item in the context menu. Anyway, let's wait until David tells us what he thinks about this issue.
The point is that I don't want my users having "delete" in the context menu. Some other OS has conditioned them to think that "delete" sends to the Recycle Bin. However, even mousemaids know that Shift-Delete (in this case Shift on the keyboard and Delete in the context menu with the mouse) bypasses the trash. This bug report just asks that the text be changed to Delete when this is the case. The actual operations should stay the same. Only change the text.
Wow, I was going to reply "unfortunately there's no way to be notified when Shift is being pressed, in order to update the text", but the blog post pointed to me KModifierKeyInfo which indeed has a keyPressed() signal. So updating the text at runtime is indeed possible.
Sounds interesting - I'll give it a try next week and probably the fix is quite trivial :-) @Bartosz: It seems I was wrong in comment #5, maybe with the help of KModifierKey the patch might be more straight forward. Sorry for this! If you'd like to have a look, please just drop me a short note.
*** Bug 183057 has been marked as a duplicate of this bug. ***
To sum up the current behavior. Pressing Shift right now is ignored when you hit the "Move to trash" link.. It simply does what it says: move to trash. (regardless of shift) I'm gonna make a patch that uses this "KModifierKeyInfo" class http://api.kde.org/4.x-api/kdelibs-apidocs/kdeui/html/classKModifierKeyInfo.html (never heard of it before) and change the menu text when shift is pressed to say "Delete" instead of "Move to trash"
SVN commit 1176493 by markg: When you view the context menu of a file/folder and shift is pressed (and the delete action line is not enabled in the settings) then the "Move to trash (del)" action gets replaced by the "Delete (shift del)" action and replaced back to "Move to trash (del)" when you release shift. BUG: 194275 M +111 -92 dolphincontextmenu.cpp M +15 -5 dolphincontextmenu.h M +8 -1 dolphinmainwindow.cpp M +5 -0 dolphinmainwindow.h WebSVN link: http://websvn.kde.org/?view=rev&revision=1176493