Bug 22675

Summary: wxWidgets bindings don't have context menu support
Product: WebKit Reporter: Diggilin <mr.diggilin>
Component: WebKit wxAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch to add wx port context menus
mr.diggilin: review-
patch for wx binding's Context Menu kevino: review+

Diggilin
Reported 2008-12-05 01:17:10 PST
wxWidgets bindings don't respond when a context menu is in order.
Attachments
patch to add wx port context menus (23.14 KB, patch)
2008-12-05 01:20 PST, Diggilin
mr.diggilin: review-
patch for wx binding's Context Menu (23.83 KB, patch)
2008-12-17 23:35 PST, Diggilin
kevino: review+
Diggilin
Comment 1 2008-12-05 01:20:21 PST
Created attachment 25766 [details] patch to add wx port context menus
Kevin Ollivier
Comment 2 2008-12-13 10:13:29 PST
When I try this patch, I get an assert in ContextMenuItem::ContextMenuItem(ContextMenuItemType type, ContextMenuAction action, const String& title, ContextMenu* subMenu). Here's the assert message: [Debug] 09:57:24: /BUILD/wxPython-src-2.8.8.1/src/common/menucmn.cpp(381): assert "parentMenu != NULL" failed in wxMenuItemBase(): menuitem should have a menu It's possible that you built in release mode and thus didn't see this assert. To build debug, you can run `WebKitTools/Scripts/set-webkit-configuration --wx --debug`. In any case, I had to give an r- to this patch because an assert like that usually indicates that on at least one platform, the approach you're trying won't work. I think what we need to do is mimic the GTK and QT ports here, and have a PlatformMenuItemDescription structure that includes a native structure, but the actual wxMenuItem(s) will be created in the ContextMenu::appendItem call. The ContextMenuItem constructors themselves will just store and return the values passed to them inside the PlatformMenuItemDescription struct. Thanks for all your work on this!
Diggilin
Comment 3 2008-12-13 15:01:11 PST
(In reply to comment #2) > When I try this patch, I get an assert in > ContextMenuItem::ContextMenuItem(ContextMenuItemType type, ContextMenuAction > action, const String& title, ContextMenu* subMenu). > > Here's the assert message: > > [Debug] 09:57:24: /BUILD/wxPython-src-2.8.8.1/src/common/menucmn.cpp(381): > assert "parentMenu != NULL" failed in wxMenuItemBase(): menuitem should have a > menu > > It's possible that you built in release mode and thus didn't see this assert. > To build debug, you can run `WebKitTools/Scripts/set-webkit-configuration --wx > --debug`. > > In any case, I had to give an r- to this patch because an assert like that > usually indicates that on at least one platform, the approach you're trying > won't work. I think what we need to do is mimic the GTK and QT ports here, and > have a PlatformMenuItemDescription structure that includes a native structure, > but the actual wxMenuItem(s) will be created in the ContextMenu::appendItem > call. The ContextMenuItem constructors themselves will just store and return > the values passed to them inside the PlatformMenuItemDescription struct. > > Thanks for all your work on this! > The default constructor for a menu item is NULL, apparently this is a wx bug that has been fixed in later versions: http://trac.wxwidgets.org/ticket/3425 Request that this be disregarded and patch reconsidered?
Diggilin
Comment 4 2008-12-17 23:35:28 PST
Created attachment 26117 [details] patch for wx binding's Context Menu Fixes a broken select statement in the previous patch Moves context menu item creation to ContextMenu::appendItem No longer uses stock wx IDs
Kevin Ollivier
Comment 5 2008-12-19 19:19:27 PST
Landed in r39416, thanks!
Note You need to log in before you can comment on or make changes to this bug.