>> On 8 Jun 2022, at 19:52, Laszlo Papp <lp...@kde.org> wrote:
>> 
>> 
>> 
>> On Tue, Jun 7, 2022 at 8:26 PM Giuseppe D'Angelo <giuseppe.dang...@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


_______________________________________________
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development

Reply via email to