Bug 22675 - wxWidgets bindings don't have context menu support
: wxWidgets bindings don't have context menu support
Status: RESOLVED FIXED
: WebKit
WebKit wx
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2008-12-05 01:17 PST by
Modified: 2008-12-19 19:19 PST (History)


Attachments
patch to add wx port context menus (23.14 KB, patch)
2008-12-05 01:20 PST, Diggilin
mr.diggilin: review-
Review Patch | Details | Formatted Diff | Diff
patch for wx binding's Context Menu (23.83 KB, patch)
2008-12-17 23:35 PST, Diggilin
kevino: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2008-12-05 01:17:10 PST
wxWidgets bindings don't respond when a context menu is in order.
------- Comment #1 From 2008-12-05 01:20:21 PST -------
Created an attachment (id=25766) [details]
patch to add wx port context menus
------- Comment #2 From 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!
------- Comment #3 From 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?
------- Comment #4 From 2008-12-17 23:35:28 PST -------
Created an attachment (id=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
------- Comment #5 From 2008-12-19 19:19:27 PST -------
Landed in r39416, thanks!