Update the ungrabMouse hack for Qt 5.8
ClosedPublic

Authored by davidedmundson on Apr 19 2017, 5:38 PM.

Details

Summary

There was an unidentified bug where when opening a context menu in
response to a mousePress event led to the next click events not being
registered. The mouse got grabbed by the item in the mousePressEvent,
but when we open a menu it didn't get released. The next click goes to
the currently grabbed item.

A workaround was added back in 2015 (and copied into various other parts
of Plasma) to ungrab the mouse after launching the context menu inside
our rightClickEvent. This doesn't work because qquickwindow code is
shuffled. It now sends the event and then grabs the mouse.

qquickwindow.cpp:653 on Qt5.8.0

Which means we're now trying to do the ungrab, before QWindow has set
our item as the grabber. That obviously doesn't work.

This real bug needs fixing in Qt, (I now know the root cause!)
but we have a workaround here already, we may as well make it work.

Note: This also needs doing in a bajillion (6) places in p-w/p-d.

FWIW, the root cause of all of these bugs is:

  • QtQuick internally keeps track of which item has "grabbed" the mouse
  • It's weirdly a singleton between windows, but that should be fine
  • It relies on the fact that when we get an X mouse press event we

should always get a mouse release event. X is surprisingly smart and
will send this even if you change window focus.

  • However - context menus are evil bastards. They do a low level grab

(completely different type of grab than the one above) and grab all
mouse events at the X level

  • Our original QtQuickItem which was handling the right click event no

longer gets our release event

  • The reason it's only somtimes is we have a "race" between being

created and the grab going through X, and the user actually releasing
the mouse.

Test Plan

Had 1 reproducible case
Added this. No longer could reproduce

Diff Detail

Repository
R242 Plasma Framework (Library)
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
davidedmundson created this revision.Apr 19 2017, 5:38 PM
Restricted Application added projects: Plasma, Frameworks. · View Herald TranscriptApr 19 2017, 5:38 PM
Restricted Application added subscribers: Frameworks, plasma-devel. · View Herald Transcript
mvourlakos added a subscriber: mvourlakos.EditedApr 19 2017, 5:52 PM

I had reported qts faulty behavior concerning contextmenus at: https://bugreports.qt.io/browse/QTBUG-59044

I think it is the mentioned case...

anthonyfieroni added a subscriber: anthonyfieroni.EditedApr 19 2017, 8:04 PM

https://phabricator.kde.org/D4712
https://phabricator.kde.org/D4711
It has one unfixible case, dismiss context menu by clicking on other window.

mart accepted this revision.Apr 20 2017, 10:12 AM
This revision is now accepted and ready to land.Apr 20 2017, 10:12 AM
mart added a comment.Apr 20 2017, 10:38 AM

do you know off hand a list of places where this is needed?
i think kicker,taskmanager,systray... anything else?

Rewrote with a runtime check and a link to a qt bug report

This revision was automatically updated to reflect the committed changes.

David, it's sould be done in plasma-framework too.