MAINT: fix type error of comparing int and KeyboardModifier

[homosapien-lcy]

Currently when running multiple demos in enable (such as enable/enable/examples/demo/enable/basic_draw.py), a TypeError: unsupported operand type(s) for &: 'int' and 'KeyboardModifier' will be raised when user moves the mouse outside the ui window.

There are two causes of this error:
1 in the new qt, keyboard related modifiers are moved to QtCore.Qt.KeyboardModifier from QtCore.Qt, thus the source of keyboard modifiers should be changed
2 in enable.qt.base_window._create_mouse_event, when the mouse is moved outside the window, the event passed to this function will be a QEvent other than an event from QGui (such as QMouseEvent or QEnterEvent). QEvent does not have any attribute related to coordinates x and y which will cause an attribute error that will be caught here https://github.com/enthought/enable/blob/main/enable/qt/base_window.py#L464. After catch, the modifiers and buttons will be changed to integer 0, which are incompatible with KeyboardModifier for & operation.

Thus the changes I implemented are:
1 Change QtCore.Qt keyboard modifiers to QtCore.Qt.KeyboardModifier keyboard modifiers
2 Set modifiers and buttons to modifiers and button classes that represent 0

I have also considered change the _create_mouse_event (https://github.com/enthought/enable/blob/main/enable/qt/base_window.py#L464) function to check whether the event is a QtGui in order to avoid entering the AttributeError:

    def _create_mouse_event(self, event):
        # If the control no longer exists, don't send mouse event
        if self.control is None:
            return None
        # If the event (if there is one) doesn't contain the mouse position,
        # modifiers and buttons then get sensible defaults.
        try:
            if getattr(event, '__module__', None).split('.')[-1] != 'QtGui':
                pos = self.control.mapFromGlobal(QtGui.QCursor.pos())
                x = pos.x()
                y = pos.y()
                modifiers = QtCore.Qt.KeyboardModifier.NoModifier
                buttons = QtCore.Qt.NoButton
            else:
                if is_qt5:
                    x = event.x()
                    y = event.y()
                else:
                    x = event.position().x()
                    y = event.position().y()
                modifiers = event.modifiers()
                buttons = event.buttons()
        except AttributeError:
            pos = self.control.mapFromGlobal(QtGui.QCursor.pos())
            x = pos.x()
            y = pos.y()
            modifiers = QtCore.Qt.KeyboardModifier.NoModifier
            buttons = QtCore.Qt.NoButton

However for two reasons I consider this undesirable:
1 Some other events that are not QtGui can also contain position information
2 In the future, other edge cases can pass in events that doesn't contain position information, thus AttributeError is actually good for handling them in general

Thus I decided to keep AttributeError as the method to handle events not containing attributes for now.
closes #1030

[corranwebster]

1 in the new qt, keyboard related modifiers are moved to QtCore.Qt.KeyboardModifier from QtCore.Qt, thus the source of keyboard modifiers should be changed

If you are making this change, you should make the corresponding changes for the mouse buttons (ie. Qt.LeftButton should probably become Qt.MouseButton.LeftButton or something similar).

2 in enable.qt.base_window._create_mouse_event, when the mouse is moved outside the window, the event passed to this function will be a QEvent other than an event from QGui (such as QMouseEvent or QEnterEvent).

Whether the event is defined in QtGui is pretty much irrelevant - what matters is the API it expresses. Don't try to implement the code you have suggested in the snippet in the description as it has the potential to break things badly in the future (eg. if things get moved out of QtGui in the future). Please remove the comment you've added, as it is misleading.

I'm not 100% sure what this is guarding against - the list of events which get routed through this code are fairly small (see

enable/enable/qt/base_window.py

Lines 140 to 174 in d99598a

	def enterEvent(self, event):
	if self._enable_window:
	self._enable_window._handle_mouse_event("mouse_enter", event)
	def leaveEvent(self, event):
	if self._enable_window:
	self._enable_window._handle_mouse_event("mouse_leave", event)
	def mouseDoubleClickEvent(self, event):
	if self._enable_window:
	name = BUTTON_NAME_MAP[event.button()]
	self._enable_window._handle_mouse_event(name + "_dclick", event)
	def mouseMoveEvent(self, event):
	if self._enable_window:
	self._enable_window._handle_mouse_event("mouse_move", event)
	def mousePressEvent(self, event):
	if self._enable_window:
	name = BUTTON_NAME_MAP[event.button()]
	self._enable_window._handle_mouse_event(name + "_down", event)
	def mouseReleaseEvent(self, event):
	if self._enable_window:
	name = BUTTON_NAME_MAP[event.button()]
	self._enable_window._handle_mouse_event(name + "_up", event)
	def wheelEvent(self, event):
	handled = False
	if self._enable_window:
	handled = self._enable_window._handle_mouse_event(
	"mouse_wheel", event)
	if not handled:
	# Allow the parent Qt widget handle the event.
	event.ignore()

) so it is possible that catching the attribute error isn't needed anymore? Or can be restricted to just the modifier/button methods?

In any case, please remove the comment you added as it is misleading.

[corranwebster]

This is a somewhat misleading comment (whether this path gets triggered is unrelated to whether the event is defined in QtGui) and the reasoning is covered by the comment in lines 453-454.

[homosapien-lcy]

Sure, I will remove it. We came up with this comment during my standup with Didrik and thought it may be useful since it shows how the attribute error can happen (by moving the mouse out of the window). What if I keep only this part:

        # The AttributeError is usually trigged when the mouse pointer
        # leaves the ui window since the event of leaving the window
        # is a "QEvent", which doesn't contain x and y positions

?

[corranwebster]

Probably should be:

Suggested change

	buttons = QtCore.Qt.NoButton
	buttons = QtCore.Qt.MouseButton.NoButton

or something similar.

[corranwebster]

Similar comment here: probably should use MouseButton.LeftButton etc.

[corranwebster]

@homosapien-lcy This is blocking getting PySide 6.4+ working for Chaco. Could you please update this?

[homosapien-lcy]

@homosapien-lcy This is blocking getting PySide 6.4+ working for Chaco. Could you please update this?

Sure, the Japan office was in public holiday last week. I will work on it now.

[homosapien-lcy]

1 in the new qt, keyboard related modifiers are moved to QtCore.Qt.KeyboardModifier from QtCore.Qt, thus the source of keyboard modifiers should be changed

If you are making this change, you should make the corresponding changes for the mouse buttons (ie. Qt.LeftButton should probably become Qt.MouseButton.LeftButton or something similar).

2 in enable.qt.base_window._create_mouse_event, when the mouse is moved outside the window, the event passed to this function will be a QEvent other than an event from QGui (such as QMouseEvent or QEnterEvent).

Whether the event is defined in QtGui is pretty much irrelevant - what matters is the API it expresses. Don't try to implement the code you have suggested in the snippet in the description as it has the potential to break things badly in the future (eg. if things get moved out of QtGui in the future). Please remove the comment you've added, as it is misleading.

I'm not 100% sure what this is guarding against - the list of events which get routed through this code are fairly small (see

enable/enable/qt/base_window.py

Lines 140 to 174 in d99598a

	def enterEvent(self, event):
	if self._enable_window:
	self._enable_window._handle_mouse_event("mouse_enter", event)
	def leaveEvent(self, event):
	if self._enable_window:
	self._enable_window._handle_mouse_event("mouse_leave", event)
	def mouseDoubleClickEvent(self, event):
	if self._enable_window:
	name = BUTTON_NAME_MAP[event.button()]
	self._enable_window._handle_mouse_event(name + "_dclick", event)
	def mouseMoveEvent(self, event):
	if self._enable_window:
	self._enable_window._handle_mouse_event("mouse_move", event)
	def mousePressEvent(self, event):
	if self._enable_window:
	name = BUTTON_NAME_MAP[event.button()]
	self._enable_window._handle_mouse_event(name + "_down", event)
	def mouseReleaseEvent(self, event):
	if self._enable_window:
	name = BUTTON_NAME_MAP[event.button()]
	self._enable_window._handle_mouse_event(name + "_up", event)
	def wheelEvent(self, event):
	handled = False
	if self._enable_window:
	handled = self._enable_window._handle_mouse_event(
	"mouse_wheel", event)
	if not handled:
	# Allow the parent Qt widget handle the event.
	event.ignore()

) so it is possible that catching the attribute error isn't needed anymore? Or can be restricted to just the modifier/button methods?
In any case, please remove the comment you added as it is misleading.

I have tried remove the attribute error catch, but then the mouse leaving the window will trigger no attribute error (enable/examples/demo/enable/basic_draw.py demo), so I guess we still needs it.

(py311) (base) cyliu@aus552cyliu enable % python3.11 enable/examples/demo/enable/basic_draw.py       
2023-05-08 18:08:35.863 Python[7392:176925] ApplePersistenceIgnoreState: Existing state will not be touched. New state will be written to /var/folders/2z/kylzj9s92y71cxscmljmpqrh0000gt/T/org.python.python.savedState
(py311) (base) cyliu@aus552cyliu enable % python3.11 enable/examples/demo/enable/basic_draw.py
2023-05-08 18:09:03.079 Python[7403:177336] ApplePersistenceIgnoreState: Existing state will not be touched. New state will be written to /var/folders/2z/kylzj9s92y71cxscmljmpqrh0000gt/T/org.python.python.savedState
Traceback (most recent call last):
  File "/Users/cyliu/Documents/3.11_test/enable/enable/examples/demo/enable/basic_draw.py", line 42, in <module>
    demo = demo_main(Demo)
           ^^^^^^^^^^^^^^^
  File "/Users/cyliu/Documents/3.11_test/enable/enable/examples/_example_support.py", line 40, in demo_main
    demo_class().configure_traits()
  File "/Users/cyliu/.venvs/py311/lib/python3.11/site-packages/traits/has_traits.py", line 2164, in configure_traits
    rc = toolkit().view_application(
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/cyliu/Documents/3.11_test/traitsui/traitsui/qt/toolkit.py", line 237, in view_application
    return view_application.view_application(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/cyliu/Documents/3.11_test/traitsui/traitsui/qt/view_application.py", line 92, in view_application
    return ViewApplication(
           ^^^^^^^^^^^^^^^^
  File "/Users/cyliu/Documents/3.11_test/traitsui/traitsui/qt/view_application.py", line 138, in __init__
    start_event_loop_qt4()
  File "/Users/cyliu/Documents/3.11_test/pyface/pyface/util/guisupport.py", line 156, in start_event_loop_qt4
    app.exec()
  File "/Users/cyliu/Documents/3.11_test/traitsui/traitsui/qt/toolkit.py", line 129, in eventFilter
    if event.type() == QtCore.QEvent.Type.KeyPress:
       ^^^^^^^^^^^^
  File "/Users/cyliu/Documents/3.11_test/traitsui/traitsui/qt/toolkit.py", line 129, in eventFilter
    if event.type() == QtCore.QEvent.Type.KeyPress:
       ^^^^^^^^^^^^
  File "/Users/cyliu/Documents/3.11_test/traitsui/traitsui/qt/toolkit.py", line 129, in eventFilter
    if event.type() == QtCore.QEvent.Type.KeyPress:
       ^^^^^^^^^^^^
  [Previous line repeated 4 more times]
  File "/Users/cyliu/Documents/3.11_test/traitsui/traitsui/qt/ui_base.py", line 175, in closeEvent
    if self._ok_to_close():
       ^^^^^^^^^^^^^^^^^^^
  File "/Users/cyliu/Documents/3.11_test/traitsui/traitsui/qt/ui_base.py", line 228, in _ok_to_close
    is_ok = not self.isModal()
                ^^^^^^^^^^^^^^
  File "/Users/cyliu/Documents/3.11_test/traitsui/traitsui/qt/toolkit.py", line 129, in eventFilter
    if event.type() == QtCore.QEvent.Type.KeyPress:
       ^^^^^^^^^^^^
  File "/Users/cyliu/Documents/3.11_test/traitsui/traitsui/qt/toolkit.py", line 129, in eventFilter
    if event.type() == QtCore.QEvent.Type.KeyPress:
       ^^^^^^^^^^^^
  File "/Users/cyliu/Documents/3.11_test/traitsui/traitsui/qt/toolkit.py", line 129, in eventFilter
    if event.type() == QtCore.QEvent.Type.KeyPress:
       ^^^^^^^^^^^^
  File "/Users/cyliu/Documents/3.11_test/enable/enable/qt/base_window.py", line 253, in paintEvent
    self.handler.paintEvent(event)
  File "/Users/cyliu/Documents/3.11_test/enable/enable/qt/base_window.py", line 88, in paintEvent
    self._enable_window._paint(event)
  File "/Users/cyliu/Documents/3.11_test/enable/enable/abstract_window.py", line 524, in _paint
    size = self._get_control_size()
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/cyliu/Documents/3.11_test/enable/enable/qt/base_window.py", line 580, in _get_control_size
    return (int(self.control.width() * self.base_pixel_scale),
                ^^^^^^^^^^^^^^^^^^^^
  File "/Users/cyliu/Documents/3.11_test/traitsui/traitsui/qt/toolkit.py", line 129, in eventFilter
    if event.type() == QtCore.QEvent.Type.KeyPress:
       ^^^^^^^^^^^^
  File "/Users/cyliu/Documents/3.11_test/traitsui/traitsui/qt/toolkit.py", line 129, in eventFilter
    if event.type() == QtCore.QEvent.Type.KeyPress:
       ^^^^^^^^^^^^
  File "/Users/cyliu/Documents/3.11_test/traitsui/traitsui/qt/toolkit.py", line 129, in eventFilter
    if event.type() == QtCore.QEvent.Type.KeyPress:
       ^^^^^^^^^^^^
  [Previous line repeated 1 more time]
  File "/Users/cyliu/Documents/3.11_test/enable/enable/qt/base_window.py", line 268, in leaveEvent
    self.handler.leaveEvent(event)
  File "/Users/cyliu/Documents/3.11_test/enable/enable/qt/base_window.py", line 146, in leaveEvent
    self._enable_window._handle_mouse_event("mouse_leave", event)
  File "/Users/cyliu/Documents/3.11_test/enable/enable/abstract_window.py", line 349, in _handle_mouse_event
    mouse_event = self._create_mouse_event(event)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/cyliu/Documents/3.11_test/enable/enable/qt/base_window.py", line 459, in _create_mouse_event
    x = event.position().x()
        ^^^^^^^^^^^^^^
AttributeError: 'PySide6.QtCore.QEvent' object has no attribute 'position'

[jwiggins]

Perhaps the more interesting question here is: Why does the signature for leaveEvent specify and argument of type QEvent and not QMouseEvent? And did something change in the behavior between Qt 5 and Qt 6?

EDIT: The check in that part of the code is if is_qt5, but since this is old code I'm guessing the change happened between Qt 4 and Qt 5 and that we want to take the is_qt5 path for all versions except Qt 4, no? That's why you're getting an AttributeError => Qt 6 is not Qt 5, so you're taking the Qt 4 code path (which is incorrect!).

[homosapien-lcy]

Perhaps the more interesting question here is: Why does the signature for leaveEvent specify and argument of type QEvent and not QMouseEvent? And did something change in the behavior between Qt 5 and Qt 6?

EDIT: The check in that part of the code is if is_qt5, but since this is old code I'm guessing the change happened between Qt 4 and Qt 5 and that we want to take the is_qt5 path for all versions except Qt 4, no? That's why you're getting an AttributeError => Qt 6 is not Qt 5, so you're taking the Qt 4 code path (which is incorrect!).

Hi John:

Good question. But I think the mouse event when leaving the window (QEvent) doesn't contain function to acquire its coordinates, thus even if we use is_qt6 to distinguish the version, we cannot get the coordinate from the event, here are the print out from the dir function for QMounseEvent vs QEvent:

<PySide6.QtGui.QMouseEvent(MouseMove pos=9,2 scn=8.901,1.72302 gbl=649,277 dev=QPointingDevice("core pointer" Mouse id=200000001000000))>
Event content:
['__new__', '__repr__', '__setattr__', '__delattr__', '__init__', 'clone', 'flags', 'globalPos', 'globalX', 'globalY', 'localPos', 'pos', 'screenPos', 'source', 'windowPos', 'x', 'y', '__doc__', '__module__', 'button', 'buttons', 'exclusivePointGrabber', 'globalPosition', 'isBeginEvent', 'isEndEvent', 'isUpdateEvent', 'position', 'scenePosition', 'setExclusivePointGrabber', 'addPassiveGrabber', 'allPointsAccepted', 'allPointsGrabbed', 'clearPassiveGrabbers', 'exclusiveGrabber', 'point', 'pointById', 'pointCount', 'pointerType', 'pointingDevice', 'points', 'removePassiveGrabber', 'setAccepted', 'setExclusiveGrabber', 'setTimestamp', 'device', 'deviceType', 'modifiers', 'setModifiers', 'timestamp', 'accept', 'ignore', 'isAccepted', 'isInputEvent', 'isPointerEvent', 'isSinglePointEvent', 'registerEventType', 'spontaneous', 'type', 'Type', '__getattribute__', '__dict__', '__hash__', '__str__', '__lt__', '__le__', '__eq__', '__ne__', '__gt__', '__ge__', '__reduce_ex__', '__reduce__', '__getstate__', '__subclasshook__', '__init_subclass__', '__format__', '__sizeof__', '__dir__', '__class__']

<PySide6.QtCore.QEvent(QEvent::Leave)>
Event content:
['__new__', '__repr__', '__setattr__', '__delattr__', '__init__', 'accept', 'clone', 'ignore', 'isAccepted', 'isInputEvent', 'isPointerEvent', 'isSinglePointEvent', 'registerEventType', 'setAccepted', 'spontaneous', 'type', '__doc__', '__module__', 'Type', '__getattribute__', '__dict__', '__hash__', '__str__', '__lt__', '__le__', '__eq__', '__ne__', '__gt__', '__ge__', '__reduce_ex__', '__reduce__', '__getstate__', '__subclasshook__', '__init_subclass__', '__format__', '__sizeof__', '__dir__', '__class__']

We can see that QMouseEvent has "position" while QEvent doesn't have anything similar. Thus we will inevitably try to find the x, y from the global coordinate like in the AttributeError block:

        except AttributeError:
            pos = self.control.mapFromGlobal(QtGui.QCursor.pos())
            x = pos.x()
            y = pos.y()
            modifiers = QtCore.Qt.KeyboardModifier.NoModifier
            buttons = QtCore.Qt.MouseButton.NoButton

[corranwebster]

Good question. But I think the mouse event when leaving the window (QEvent) doesn't contain function to acquire its coordinates, thus even if we use is_qt6 to distinguish the version

There are two issues here:

• QEnterEvent/QLeaveEvent in Qt5 have a subtly different API for accessing mouse coordinates than QMouseEvent; this was fixed in Qt6 (both are now subclasses of QSinglePointEvent), but the API was changed.
• More importantly, QEnterEvent/QLeaveEvent don't have button state associated with them (and also no modifier key in Qt5), so the attribute error is going to be on those methods - the code to get the position is essentially fine.

There may be deeper history with Qt4 about why this code was written this way, but I'm not going to dig into that right now,

We also need to take into account QWheelEvent, which is the 3rd type of mouse event which this code handles, but I think it's close enough to QMouseEvent on all versions of Qt that it's fine.

[corranwebster]

This isn't perfect, but I will merge now to get Chaco unstuck and may make a separate PR to revise the logic to account for the observations above.