Bug in event handling in wx.ogl.Shape

[Tried to post this via gmane, but it bounced. The error is a bit
odd, so apologies if I've misunderstood and am now posting a
duplicate]

Hi,

A small bug, I think. In the methods below, which are part of Shape
in
ogl.__basic, the return from HitTest should be first tested against a
boolean,
because it can return False (see similar code elsewhere in ogl).

    def OnLeftClick(self, x, y, keys = 0, attachment = 0):
        if self._sensitivity & OP_CLICK_LEFT != OP_CLICK_LEFT:
            if self._parent:
                attachment, dist = self._parent.HitTest(x, y)
                self._parent.GetEventHandler().OnLeftClick(x, y, keys,
attachment)

    def OnRightClick(self, x, y, keys = 0, attachment = 0):
        if self._sensitivity & OP_CLICK_RIGHT != OP_CLICK_RIGHT:
            attachment, dist = self._parent.HitTest(x, y)
            self._parent.GetEventHandler().OnRightClick(x, y, keys,
attachment)

So it should look more like:

    def OnLeftClick(self, x, y, keys = 0, attachment = 0):
        if self._sensitivity & OP_CLICK_LEFT != OP_CLICK_LEFT:
            if self._parent:
                hit = self._parent.HitTest(x, y)
                if hit:
                    attachment, dist = hit
                self._parent.GetEventHandler().OnLeftClick(x, y, keys,
attachment)

(note the call to the parent still happens even if hit is False - I am
not sure
this is correct in this case, but it seems to be what other, similar
code does).

Also, I do have a case where this causes an exception to be thrown
(which is how
I found the bug), so it's not guaranteed to always be true. Please
email me if
you require more info as I'm making a one-off post from gmane.

Cheers,
Andrew

andrew cooke wrote:

[Tried to post this via gmane, but it bounced. The error is a bit
odd, so apologies if I've misunderstood and am now posting a
duplicate]

Hi,

A small bug, I think. In the methods below, which are part of Shape
in
ogl.__basic, the return from HitTest should be first tested against a
boolean,
because it can return False (see similar code elsewhere in ogl).

    def OnLeftClick(self, x, y, keys = 0, attachment = 0):
        if self._sensitivity & OP_CLICK_LEFT != OP_CLICK_LEFT:
            if self._parent:
                attachment, dist = self._parent.HitTest(x, y)
                self._parent.GetEventHandler().OnLeftClick(x, y, keys,
attachment)

    def OnRightClick(self, x, y, keys = 0, attachment = 0):
        if self._sensitivity & OP_CLICK_RIGHT != OP_CLICK_RIGHT:
            attachment, dist = self._parent.HitTest(x, y)
            self._parent.GetEventHandler().OnRightClick(x, y, keys,
attachment)

So it should look more like:

    def OnLeftClick(self, x, y, keys = 0, attachment = 0):
        if self._sensitivity & OP_CLICK_LEFT != OP_CLICK_LEFT:
            if self._parent:
                hit = self._parent.HitTest(x, y)
                if hit:
                    attachment, dist = hit
                self._parent.GetEventHandler().OnLeftClick(x, y, keys,
attachment)

(note the call to the parent still happens even if hit is False - I am
not sure
this is correct in this case, but it seems to be what other, similar
code does).

Also, I do have a case where this causes an exception to be thrown
(which is how
I found the bug), so it's not guaranteed to always be true. Please
email me if
you require more info as I'm making a one-off post from gmane.

Please use svn diff or some other diff tool and provide an actual patch for your proposed changes. (Unified context diff if possible.)

···

--
Robin Dunn
Software Craftsman

OK, here's an actual patch (there seems to be no way to add
attachments to google groups). I fixed a couple more occurrences that
also made no test for False, although I don't call those in my code.

diff -r -u wxPython-original/wx/lib/ogl/_basic.py wxPython/wx/lib/ogl/
_basic.py
--- wxPython-original/wx/lib/ogl/_basic.py 2008-07-19
00:38:50.000000000 -0400
+++ wxPython/wx/lib/ogl/_basic.py 2009-09-01 11:45:00.000000000
-0400
@@ -990,12 +990,16 @@
     def OnLeftClick(self, x, y, keys = 0, attachment = 0):
         if self._sensitivity & OP_CLICK_LEFT != OP_CLICK_LEFT:
             if self._parent:
- attachment, dist = self._parent.HitTest(x, y)
+ hit = self._parent.HitTest(x, y)
+ if hit:
+ attachment, dist = hit
                 self._parent.GetEventHandler().OnLeftClick(x, y,
keys, attachment)

     def OnRightClick(self, x, y, keys = 0, attachment = 0):
         if self._sensitivity & OP_CLICK_RIGHT != OP_CLICK_RIGHT:
- attachment, dist = self._parent.HitTest(x, y)
+ hit = self._parent.HitTest(x, y)
+ if hit:
+ attachment, dist = hit
             self._parent.GetEventHandler().OnRightClick(x, y, keys,
attachment)

     def OnDragLeft(self, draw, x, y, keys = 0, attachment = 0):
@@ -1090,14 +1094,18 @@
     def OnBeginDragRight(self, x, y, keys = 0, attachment = 0):
         if self._sensitivity & OP_DRAG_RIGHT != OP_DRAG_RIGHT:
             if self._parent:
- attachment, dist = self._parent.HitTest(x, y)
+ hit = self._parent.HitTest(x, y)
+ if hit:
+ attachment, dist = hit
                 self._parent.GetEventHandler().OnBeginDragRight(x, y,
keys, attachment)
             return

     def OnEndDragRight(self, x, y, keys = 0, attachment = 0):
         if self._sensitivity & OP_DRAG_RIGHT != OP_DRAG_RIGHT:
             if self._parent:
- attachment, dist = self._parent.HitTest(x, y)
+ hit = self._parent.HitTest(x, y)
+ if hit:
+ attachment, dist = hit
                 self._parent.GetEventHandler().OnEndDragRight(x, y,
keys, attachment)
             return

···

On Sep 1, 12:53 am, Robin Dunn <ro...@alldunn.com> wrote:

andrew cooke wrote:
> [Tried to post this via gmane, but it bounced. The error is a bit
> odd, so apologies if I've misunderstood and am now posting a
> duplicate]

> Hi,

> A small bug, I think. In the methods below, which are part of Shape
> in
> ogl.__basic, the return from HitTest should be first tested against a
> boolean,
> because it can return False (see similar code elsewhere in ogl).

> def OnLeftClick(self, x, y, keys = 0, attachment = 0):
> if self._sensitivity & OP_CLICK_LEFT != OP_CLICK_LEFT:
> if self._parent:
> attachment, dist = self._parent.HitTest(x, y)
> self._parent.GetEventHandler().OnLeftClick(x, y, keys,
> attachment)

> def OnRightClick(self, x, y, keys = 0, attachment = 0):
> if self._sensitivity & OP_CLICK_RIGHT != OP_CLICK_RIGHT:
> attachment, dist = self._parent.HitTest(x, y)
> self._parent.GetEventHandler().OnRightClick(x, y, keys,
> attachment)

> So it should look more like:

> def OnLeftClick(self, x, y, keys = 0, attachment = 0):
> if self._sensitivity & OP_CLICK_LEFT != OP_CLICK_LEFT:
> if self._parent:
> hit = self._parent.HitTest(x, y)
> if hit:
> attachment, dist = hit
> self._parent.GetEventHandler().OnLeftClick(x, y, keys,
> attachment)

> (note the call to the parent still happens even if hit is False - I am
> not sure
> this is correct in this case, but it seems to be what other, similar
> code does).

> Also, I do have a case where this causes an exception to be thrown
> (which is how
> I found the bug), so it's not guaranteed to always be true. Please
> email me if
> you require more info as I'm making a one-off post from gmane.

Please use svn diff or some other diff tool and provide an actual patch
for your proposed changes. (Unified context diff if possible.)

--
Robin Dunn
Software Craftsmanhttp://wxPython.org

Hi Andrew,

···

On Sep 1, 10:53 am, andrew cooke <and...@acooke.org> wrote:

OK, here's an actual patch (there seems to be no way to add
attachments to google groups). I fixed a couple more occurrences that
also made no test for False, although I don't call those in my code.

You can add attachments, you just have to send them to the email
address:

wxpython-users@googlegroups.com

Make sure you send the email from the address you used when you signed
up for this group.

-------------------
Mike Driscoll

Blog: http://blog.pythonlibrary.org

andrew cooke wrote:

OK, here's an actual patch

Thanks.

···

--
Robin Dunn
Software Craftsman