Bug 307747 - Dolphin does not respect QApplication::startDragTime() when starting a drag
Summary: Dolphin does not respect QApplication::startDragTime() when starting a drag
Status: RESOLVED FIXED
Alias: None
Product: dolphin
Classification: Applications
Component: view-engine: general (show other bugs)
Version: 2.1
Platform: unspecified Linux
: NOR wishlist
Target Milestone: ---
Assignee: Dolphin Bug Assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-02 19:58 UTC by Franz Trischberger
Modified: 2017-11-11 14:11 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Franz Trischberger 2012-10-02 19:58:52 UTC
The distance when the drag gets started is too short, the timeout, too.
It happens a lot at a friend of mine. Working as usual, a little shake in the hand while clicking to enter a folder, and dolphin tells him "Sorry, you cannot drop a folder on itself". As he did not drop anything but simply click and get an error he closes dolphin and reopens it :/
The drag indicator is not visible, too, so there seems to be sort of a timeout. Could that be applied here, too?

Reproducible: Always
Comment 1 Frank Reininghaus 2012-10-03 08:34:44 UTC
Thanks for the report!

A drag is started as soon as the distance between the position where the mouse button has been pressed and the current position is larger than QApplication::startDragDistance(), which is a global setting that can be configured in the System Settings (Hardware/Input Devices -> Mouse -> Advanced). Just tell your friend to increase that value ;-)

The System Settings module also mentions a drag timeout, but AFAIK, we don't use that in Dolphin at the moment. I don't know why this has not been implemented.

Do you think that increasing the drag start distance manually is sufficient for your use case, or should we keep this report open as a reminder to reconsider the implementation of the drag start time?
Comment 2 Franz Trischberger 2012-10-03 09:19:29 UTC
I totally missed that it is possible to set a drag distance...
Nevertheless there seem to be 2 issues:
* I set it to 10, but it actually starts the drag when at 9
* diagonal drag seems to simply add vertical and horizontal pixels for the drag distance calculation. The result is a diagonal of 4-5 pixel leading to a drag (too short, again).
I did not check the code but use kwin magnifier and count pixels ;)

Having the drag timeout implemented would be great. It defaults to 500ms and a single click usually is done in <150ms. The drag indicator as a visual confirmation of the action in progress is IMHO more user friendly - it currently does not appear when the drag distance was reached but not the timeout.
Comment 3 Frank Reininghaus 2012-10-03 12:02:46 UTC
(In reply to comment #2)
> * I set it to 10, but it actually starts the drag when at 9

Do you mean when there are 9 pixels between the two points? In that case, the mouse has actually moved by 10 pixels.

> * diagonal drag seems to simply add vertical and horizontal pixels for the
> drag distance calculation. The result is a diagonal of 4-5 pixel leading to
> a drag (too short, again).
> I did not check the code but use kwin magnifier and count pixels ;)

The code is [in KItemListController::mouseMoveEvent(QGraphicsSceneMouseEvent* event, const QTransform& transform)]:

if ((pos - m_pressedMousePos).manhattanLength() >= QApplication::startDragDistance()) {
    // ...
    startDragging();
}

The "Manhattan length", which is traditionally used to measure the distance for drag events, indeed adds the "horizontal" and "vertical" distances, see

http://qt-project.org/doc/qt-4.8/qpoint.html#manhattanLength

> Having the drag timeout implemented would be great.

I've just had a look at the code in QAbstractItemView. It seems that even there, the drag timeout is ignored, which is probably why it hasn't been implemented in Dolphin's new view engine either. But in any case, I'll update the summary of this report.

But if we implement this, I can already see people who make drags&drops really fast report the new behaviour as a bug :-(

> The drag indicator as a visual
> confirmation of the action in progress is IMHO more user friendly - it
> currently does not appear when the drag distance was reached but not the
> timeout.

Cannot confirm that. As soon as the drag distance is reached, the indicator (I think you mean the icon shown next to the mouse pointer) is shown for me.
Comment 4 Franz Trischberger 2012-10-06 08:57:44 UTC
> Do you mean when there are 9 pixels between the two points? In that case,
> the mouse has actually moved by 10 pixels.
Yes, that's what I meant, but I was wrong :( reading error when using kruler.
Distance is correct...

> The "Manhattan length", which is traditionally used to measure the distance
> for drag events, indeed adds the "horizontal" and "vertical" distances, see
Ah, OK. So all fine here, too.


> But if we implement this, I can already see people who make drags&drops
> really fast report the new behaviour as a bug :-(
But they at least have a chance by lowering the drag timeout to something they can work with.

> 
> > The drag indicator as a visual
> > confirmation of the action in progress is IMHO more user friendly - it
> > currently does not appear when the drag distance was reached but not the
> > timeout.
> 
> Cannot confirm that. As soon as the drag distance is reached, the indicator
> (I think you mean the icon shown next to the mouse pointer) is shown for me.
OK... You are right.
It's just when this issue appears - lower your drag distance, shake your hand while clicking. In that case there is no visual feedback that there actually was a drag. Click "slower" and the indicator starts appearing (it happens to be still translucent -is there a fade-in-animation?)
Comment 5 Thomas Lübking 2012-12-12 22:17:57 UTC
Sorry for hooking onto this - i'd try to fix that from the other side.
The problem is not so much, that you accidentally started a drag, when attempting to click, but that the click is not performed and you get a notice instead.

Since it it actually pretty unlikely that someone intentionally tries to drop a folder onto itself i'd instead of just warn
1. check whether this is an internal drop ("do we drag this")
2. measure drag time and / or maximum distance ("did the user forget what he's dragging?" "does he just try to cancel the action by placing the item where it was?")

If it's an internal drag and time and distance is "not that much" (tm), just treat it as a "dirty" click (no problem, since that action is easily reverted)

If it's not an internal drag and/or time or distance is "quite some" (tm), inform the user that it's not possible to drop a folder on itself and that s/he can cancel DnD ops by pressing "Esc" anytime, resp. can still cancel after the drop and also about setting the drag distance.

Just my 2¢ - if you'd accept such patch, i volunteer to write it (minus new strings if and for  4.10)
Comment 6 Frank Reininghaus 2012-12-13 07:03:28 UTC
Thanks Thomas for your suggestion. I'm not entirely sure though if I get what exactly you mean.

First I thought you mean that we should always consider 'internal drags' not as drags, but as clicks. I admit that this makes more sense the longer I think about it, even though the current behaviour (start a drag as soon as the minimum drag distance is reached) might be more 'correct'.

That could be implemented in a quite straightforward way, I think.

But then..

(In reply to comment #5)
> If it's an internal drag and time and distance is "not that much" (tm), just
> treat it as a "dirty" click (no problem, since that action is easily
> reverted)

This is what we already do - if the distance is "not that much" (i.e., smaller than QApplication::startDragDistance()), there is no drag, but a click once the button is released.

> If it's not an internal drag and/or time or distance is "quite some" (tm),

This is the case in which we start a drag currently (except if it not an 'internal drag', but the distance is small, but I think that this rarely happens).

But I guess that you meant what I said above, and I think it won't hurt if we do something like that. It would be nice if you could provide a patch!
Comment 7 Thomas Lübking 2012-12-13 21:23:02 UTC
(In reply to comment #6)
> Thanks Thomas for your suggestion. I'm not entirely sure though if I get
> what exactly you mean.
> 
> First I thought you mean that we should always consider 'internal drags' not
> as drags, but as clicks.

More or less, see https://git.reviewboard.kde.org/r/107708/

> think about it, even though the current behaviour (start a drag as soon as
> the minimum drag distance is reached) might be more 'correct'.
The difference here is that usually the DnD is tagged "forbidden" and simply nothing happens - you just retry the click.
In Dolphin atm. you get a persistent warning that (actually worse) also shift the UI under the cursor.
 
> That could be implemented in a quite straightforward way, I think.
Yesno - pretty much. See  RR ;-)

> This is what we already do - if the distance is "not that much" (i.e.,
> smaller than QApplication::startDragDistance()), there is no drag, but a
> click once the button is released.
I was referring to the time and distance during the DnD - not before.
It's like you get some extra timeout (because usually you'll need a short while to do anything reasonable because you've to reposition the mouse etc.)
Comment 8 Nate Graham 2017-10-18 21:26:44 UTC
FWIW a patch to resolve this is currently working its way through the review process: https://phabricator.kde.org/D6281
Comment 9 Elvis Angelaccio 2017-11-11 14:11:26 UTC
Git commit 99e80c1c7e6e77aa26ccbca4fbb0430b35974544 by Elvis Angelaccio, on behalf of Emirald Mateli.
Committed on 11/11/2017 at 14:06.
Pushed by elvisangelaccio into branch 'master'.

Prevent folders from drag and dropping onto themselves in dolphin main view

Summary:
This patch aims to improve user experience by not allowing the user to drag and drop a folder into itself.

The current behavior shows a red message at the top which can then be closed by the user, instead of relying on that, this patch disables the option of dropping onto self and uses the "Invalid drop target cursor" to highlight the behavior.

Since spectacle is unable to screenshot the cursor overlay, find attached a photo of the screen.
{F3787651}

Test Plan:
1. Drag a folder.
2. Drop it onto itself.

Reviewers: #dolphin, elvisangelaccio, ngraham, rkflx, dfaure

Reviewed By: #dolphin, elvisangelaccio, rkflx, dfaure

Subscribers: rkflx, ngraham, elvisangelaccio, dfaure, anthonyfieroni, #konqueror

Tags: #dolphin

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

M  +1    -1    src/kitemviews/kfileitemmodel.h
M  +10   -1    src/kitemviews/kitemlistcontroller.cpp
M  +14   -0    src/kitemviews/kitemmodelbase.cpp
M  +14   -0    src/kitemviews/kitemmodelbase.h
M  +6    -0    src/panels/places/placesitemmodel.cpp
M  +1    -0    src/panels/places/placesitemmodel.h
M  +2    -0    src/tests/CMakeLists.txt
A  +81   -0    src/tests/draganddrophelpertest.cpp     [License: GPL (v2+)]
M  +12   -0    src/views/draganddrophelper.cpp
M  +9    -2    src/views/draganddrophelper.h

https://commits.kde.org/dolphin/99e80c1c7e6e77aa26ccbca4fbb0430b35974544