On 07/21/2012 01:34 PM, Thomas Lübking wrote:
> Am 21.07.2012, 00:48 Uhr, schrieb Thomas Gahr <[email protected]>:
>
> Someone first had to fall out of the beds - i hope that's the weather
> and not age ...
>
>> It shows the default cursor if no modifier is pressed, since in this
>> case the user will have to pick what action to take upon dropEvent -
>> i.e. during drag the action is undefined.
> GUI wise certainly a valid concern - one could pot. also abuse
> Qt::WhatsThisCursor?
>
> Anyway.
>
>> It seems a little strange to me, though. It changes the "override
>> cursor" and I would've expected sth like this:
>>
>> void MailBoxTreeView::dragMoveEvent(QDragMoveEvent *event)
>> {
>> QTreeView::dragMoveEvent(event);
>>
>> if (!event->isAccepted()) {
>> return;
>> }
>> if (event->keyboardModifiers() == Qt::ShiftModifier) {
>> event->setDropAction(Qt::MoveAction);
>> } else if (event->keyboardModifiers() == Qt::ControlModifier) {
>> event->setDropAction(Qt::CopyAction);
>> } else {
>> // Qt's default is to show a cursor indicating copy-action
>> but we don't want that
>> // because the action is undefined until the user has chosen
>> if(qApp->overrideCursor())
>> qApp->overrideCursor()->setShape(Qt::ArrowCursor);
>> }
>> }
>>
>> to work. But instead this variant changes the cursor back ot
>> ArrowCursor for good. Meaning: even if the user moves the mouse over
>> a region that changes state - i.e. from "drop allowed here" to "no
>> drop allowed here" - the cursor stays Qt::ArrowCursor.
>
> Yes - no.
>
> Background.
>
> First off: it is *not* a legal action to control the cursor from the
> drop target.
>
> This will probably not work on windows at all and will not work on X11
> unless source and target are in the same process (Qt grabs the cursor
> for the drag and by this other applications can not access it anymore)
> It does not mean it's not possible, but be aware of the limits.
Oh boy, I had a feeling that this might cause trouble on windows.
I assumed that DnD operations only happen inside Trojita, not sure if
accepting drops from another app makes sense and is a valid use case.
>
> Next issue is that you "steal" the shape of one (of the) static
> QCursor(s) used as the override by the dragmanager - it will continue
> to use it no matter of the shape you used and it will do so on each
> and every following drag & drop action (unless the dragmanager cleans
> up, what will unlikely happen directly after the drop)
I see. Bad idea. Well, directly calling setShape on the cursor returned
by overrideCursor was the first, naive idea. So no surprise it was wrong .
>
> What your patch solution does is to restore the override cursor if
> there's not defined action.
> This will work iff there's only one override cursor on the stack, for
> the DragManagers following changeOverrideCursor() now fails.
>
> The pitfall here is that DnD runs in a nested event loop, ie. there
> *could* be more override cursors on the stack (for other purposes) in
> which case you should not just restore them. So the "correct" approach
> would be to test:
>
> QCursor *prevOverride = qApp->overrideCursor();
> qApp->restoreOverrideCursor();
> if (qApp->overrideCursor()) // there's still one on the stack
> "whil'ing" them away might cause random trouble -> give up
> qApp->setOverrideCursor(*prevOverride);
>
> when you want to get rid of the + icon and just establish _any_
> override cursor otherwise (ie. set any one but only if there's none)
> to allow the dragmanager to alter it in it's normal process (ie. you
> still have to return the proper drop action)
You're right, I am completely ignoring the possibility that there are
more than one cursors on the stack. Will do that if this change is still
wanted.
Though only the following version does not make the app crash:
QCursor prevOverride = *qApp->overrideCursor();
qApp->restoreOverrideCursor();
if (qApp->overrideCursor())
qApp->setOverrideCursor(prevOverride);
...as overrideCursor return the address of the tip of an intern
QList<QCursor> and restoreOverrideCursor pops from that list.
>
> Notice again that this is *NOT* "legal" action and does only work for
> "internal" (inside Trojitá) DnD operations and will potentially not
> work on windows at all.
> You're cheating on the dragmanagers implementation details here.
In my original implementation of MailBoxTreeView I did not bother
messing with the cursor, I just immitated Dolphin's behaviour, which is
to keep the + icon.
I guess if it was that easy to implement this correctly, KDE would've
done it?
So seeing that this is potentially causing trouble it's up to Jan to
decide if this change is still wanted. Jan?
>
> (And no, abusing and altering the shape for the LinkAction cursor
> won't work in that case either ;-)
Yeah, thinking about that idea makes shrug, though I give myself half a
credit for creativity ;)
>
>
>> Oh if only someone who's probably read all QWidget sources was on
>> this mailing list... *cough* ;)
> FTR: nothing of this can be found in a QWidget based class, but that's
> another story ;-)
I figured if you're firm with QWidgets you might have good knowledge of
Qt's gui code in general - and it seems I was right :)
>
> Cheers,
> Thomas
>
