Opened 10 years ago

Closed 6 years ago

#4147 closed bug (fixed)

Magnify app crashes

Reported by: atmartens Owned by: jscipione
Priority: normal Milestone: R1
Component: Kits/Interface Kit Version: R1/pre-alpha1
Keywords: Cc: stimut@…
Blocked By: Blocking: #6610
Has a Patch: no Platform: All

Description

I was playing with the magnifying application and it crashed on me after a right-click.

Here is a backtrace:

#0  0x0020b204 in TMagnify::MouseDown ()
#1  0x0036c2d0 in BWindow::DispatchMessage () from /boot/system/lib/libbe.so
#2  0x00368ad3 in BWindow::task_looper () from /boot/system/lib/libbe.so
#3  0x002ba904 in BLooper::_task0_ () from /boot/system/lib/libbe.so
#4  0x0062f08d in thread_entry () from /boot/system/lib/libroot.so
#5  0x78074fec in ?? ()

Attachments (2)

magnify-crash.png (252.5 KB ) - added by atmartens 10 years ago.
Menu.cpp.diff (449 bytes ) - added by stimut 10 years ago.
Updated fix

Download all attachments as: .zip

Change History (15)

by atmartens, 10 years ago

Attachment: magnify-crash.png added

comment:1 by anevilyak, 10 years ago

Component: - GeneralApplications/Magnify

comment:2 by atmartens, 10 years ago

The only problem is I can't seem to reproduce the bug. I'll keep at it. :-)

comment:3 by stimut, 10 years ago

I was able to reproduce it: select a separator from the pop-up menu.

This is caused by the BPopupMenu::Go() function returning non-NULL when an item is selected even if the item has no BMessage associated with it (i.e. the item does nothing - such as for a separator).

I have created a simple patch so that it now returns NULL when the item selected has no message associated with it.

comment:4 by stimut, 10 years ago

Cc: stimut@… added

comment:5 by stippi, 10 years ago

Nice work! You definitely identified the problem and correct location for the fix, only your fix is incorrect. BMenu should not care whether the BMenuItem has a BMessage or not, but I don't suppose it should return BSeparatorMenuItem instances. This should actually be checked against the BeOS implementation. I still have a ZETA installation which I can test on, but in general, I think one should return NULL in case dynamic_casting to BSeparatorMenuItem is successful.

in reply to:  5 comment:6 by jackburton, 10 years ago

Replying to stippi:

Nice work! You definitely identified the problem and correct location for the fix, only your fix is incorrect. BMenu should not care whether the BMenuItem has a BMessage or not, but I don't suppose it should return BSeparatorMenuItem instances. This should actually be checked against the BeOS implementation. I still have a ZETA installation which I can test on, but in general, I think one should return NULL in case dynamic_casting to BSeparatorMenuItem is successful.

I seem to recall something like this having been fixed in the past: IOW: this chould be a regression.

comment:7 by jackburton, 10 years ago

Check hrev17026.

comment:8 by stippi, 10 years ago

Ah, that makes sense. The correct fix is thus "if (item->Frame().Contains(where) && item->IsEnabled())".

in reply to:  8 comment:9 by stimut, 10 years ago

Component: Applications/MagnifyKits/Interface Kit

Replying to stippi:

Ah, that makes sense. The correct fix is thus "if (item->Frame().Contains(where) && item->IsEnabled())".

Yes, I agree. I have updated the patch in case someone wants to commit the changes.

by stimut, 10 years ago

Attachment: Menu.cpp.diff added

Updated fix

comment:10 by stippi, 10 years ago

Resolution: fixed
Status: newclosed

Thanks a lot! Applied in hrev35248.

comment:11 by jscipione, 6 years ago

Blocking: 6610 added
Resolution: fixed
Status: closedreopened

This fix for this bug was not right because it caused #6610

comment:12 by jscipione, 6 years ago

Owner: changed from axeld to jscipione
Status: reopenedin-progress

comment:13 by jscipione, 6 years ago

Resolution: fixed
Status: in-progressclosed

Fixed in hrev45977

Note: See TracTickets for help on using tickets.