[Development] QPushButton: drag and drop

[Volker Hilsheimer]

>> On 8 Jun 2022, at 19:52, Laszlo Papp <lpapp at kde.org> wrote:
>> 
>> 
>> 
>> On Tue, Jun 7, 2022 at 8:26 PM Giuseppe D'Angelo <giuseppe.dangelo at kdab.com> wrote:
>> Il 07/06/22 20:57, Laszlo Papp ha scritto:
>> > Just checked the Qt wiki, but it does not seem to speak about this rule. 
>> > Only binary and source compatibility. No behaviour compatibility. And by 
>> > the way, fixing the bug to match the documentation and therefore 
>> > intended behaviour would not even be behaviour break.
>> > 
>> > https://wiki.qt.io/Qt-Version-Compatibility 
>> > <https://wiki.qt.io/Qt-Version-Compatibility>
>> 
>> There's behaviour break and behaviour break.
>> 
>> Here we're talking about a fundamental change to how Qt has (not?) 
>> handled mouse release notifications on DND. I can wager there's also 
>> other many other similar situations (e.g. while having the mouse grab, a 
>> widget gets hidden, or a modal window opens up, etc.) that are currently 
>> similarly unhandled. Bonus points for "user cancels DND with the 
>> keyboard (escape) and releases into the widget", "a modal window opens 
>> up and closes down while the user never releases the mouse", etc.
>> 
>> 
>> I'm calling it a "fundamental" change, because
>> 
>> * any widget currently handling DND already does not expect to receive 
>> any event when DND is done; this may cause a further bug, for instance a 
>> widget gets a mouse release when it thinks it never saw the 
>> corresponding press;
>> 
>> * any widget currently NOT handling DND (e.g. QPushButton) needs to 
>> handle the fake release event, in whatever way it's delivered.
>> 
>> which is a long way to say "any QWidget subclass handling mouse events 
>> needs to be changed". And that's a very high bar for a behavioral change.
>> 
>> 
>> If anything, this could start on a opt-in basis (a widget attribute set 
>> by the user, possibly also an application attribute, if you're feeling 
>> adventurous) and slowly starting fixing any misbehaving widget in the 
>> span of 2-3 minor releases, then we flip the attribute for next LTS (opt 
>> out instead of in), then it's the default in Qt 7.
>> 


Let’s put things into a perspective here. You are proposing a flag that we perhaps flip on by default within Qt 6, and that might break a whole ton of widgets out there that today correctly implement drag’n’drop and that will start failing in mysterious, and largely untestable (because testing drag’n’drop requires special tools) ways. “Adventurous” indeed :P

We are having this discussion is because a widget doesn’t get a QEvent::MouseButtonRelease in symmetry with an earlier MouseButtonPress. As I pointed out earlier, a widget also won’t get a symmetrical MouseButtonRelease when a modal dialog opens in response to press; the release gets eaten by the dialog. (And we have bugs with enter/leave like the one Gatis pointed at, which in my mind are orthogonal to this, and is not addressed by any of the proposals made in this thread so far anyway; but those would be good bugs to fix in order to familiar with the complexity of the code involved here).

You are suggesting that all widgets in Qt, and possibly thousands of custom widget implementations out there, have to start adding special code to their mouseReleaseEvent, ignoring specially flagged events because someone might 

* subclass that widget class and exec() a QDrag or a QDialog in mousePressEvent
* or connect to a pressed() signal (if that exists) and do the same

And since the argument is “we must be symmetrical with our events so widgets don’t remain in a pressed state”, we cannot really fix the one (QDrag) and not the other (QDialog). If a widget author cannot be expected to maintain their widget state before or after calling QDrag::exec in a mousePressEvent handler, then they cannot be expected to manage their widget state before or after calling QDialog::exec, either.

So, if we deliver the release at the end (or beginning?) of the QDrag, then we have to deliver the release when a modal dialog opens (or closes?), because these are the exact same things (user interface enters a different mode, so events get eaten and handled by that mode, which results in asymmetry). And I am not at all convinced that we should send a QEvent::MouseButtonRelease to a widget that opens a modal dialog in their mousePressEvent handler. I’d consider that a bug, breaking the meaning of “modal”. And a drag’n’drop, as it stands today, is modal (no dialog and window activation state as the visual cue, but instead the mouse cursor shows that you are dragging something; your desktop will behave differently, maybe give you other visual cues).

Perhaps you don’t agree that QDrag::exec starts a modal user interface state similar to a modal dialog? Then the next question is whether we need to deal with key events during the drag. Should we send shift/alt/control key presses to the widget as well? Today those are entirely handled by the drag’n’drop machinery to switch the drag operation. How about wheeling the mouse during a drag? Either we are modal (events get eaten) or we are not (events get processed by the grabbing or focus widget). Today we are modal. Changing that is a huge change, and IMHO a wrong change, also deviating from how every desktop environment seems to think about “drag and drop”.


> I am not asking for a fake release event myself, just the usual one. If someone manages the state restore manually, that would not be broken by the usual release happening as far as I can see.

You are asking for a release event to be delivered to a widget, even though the release event was already handled (and eaten) by the modal state. That’s a fake/synthesized event in my book. And if we deliver such an event, then we have to deliver the event to any widget that starts a drag. And since by overriding mousePressEvent, any widget can start a drag, all widgets have to handle that new release event, if only by ignoring it if the source is some new SynthesizedByQtForReasons flag.

FWIW, there are a couple of widget classes today that implement special mouse event handling based on the event’s source. The vast majority of widget classes doesn’t.

> I was thinking of drag and drop setting an internal state for the QWidget which then the QWidget can use to decide whether to emit clicked() or not to address Volker's concern?

Which in the case of QAbstractButton means a change to the mouseReleaseEvent implementation, testing for that flag being set, and the event’s source flag being set, and then maybe not emitting clicked().

So just because you are adding drag support to your QPushButton subclass and don’t want to maintain the button’s state yourself if a drag is started (or finished), the author of QAbstractButton now needs to somehow deal with this.


> In worst case, if this idea is not viable, then sure, I still think it is better to go for opt-in than what we have today (or have had for 12 years at least). We have to start somewhere, yes.


As long as we don’t have a non-blocking way to start a QDrag, anything you can come up with will be significantly more complex than the widget fixing it’s own state (by calling setDown(false) in the case of a QAbstractButton) after calling QDrag::exec.

And even if we had some non-blocking way to start a drag’n’drop operation, like we have modalDialog->show(), I’m still rather convinced that drag’n’drop is a modal operation that should handle and eat the release event. Just as showing a modal dialog via QDialog::show is still a modal operation where the dialog will eat the release.


I conceptually like the idea of delivering a “QEvent::GrabCanceled” event to the widget that loses the grab due to a modal operation (modal dialog or drag) starting. Widgets that want to, can then implement "cancelable interaction" by handling that event. Like QPushButton does when you press and then move the mouse out of the button - the button stops being pressed, releasing now will not click. There’s code for that in QPushButton. No need for flags, and no need for all other widget classes to do anything at all. They will just keep working as they do today.

But as long as blocking QDrag::exec is the only way to start a drag, delivering such an event adds nothing useful, at least for that particular scenario. Widgets that don’t have a resettable state won’t handle it; widgets that don’t implement drag’n’drop and don’t open modal dialogs won’t handle it (because why should they?). Widgets that do support dragging today do already handle their state before or after calling QDrag::exec. So you have to subclass and do the work yourself if you want to add drag’n’drop support to e.g. a button. And then you have to override another event handler to call QAbstractButton::setDown(false), instead of simply doing it before or after calling QDrag::exec.

Perhaps it’s worth starting to explore how to implement a non blocking QDrag::start on Windows, macOS, iOS, and Android (on X it’s probably trivial). Knowing what I know about at least Windows and macOS, it’s going to be a challenge. But also a good way to get familiar with that code and what’s involved on the different operating systems. It would still be a modal operation though, just as showing a modal QDialog is a modal operation.

Once we have that, then we anyway need an event that informs the source of the drag that the drag has been finished, with a certain operation (move vs copy vs link vs cancelled) so that the source widget can take care of its side of the operation (remove the data if it was a move). And then you can call setDown(false) when handling that event.


Volker