| Summary: | [GTK] ContextMenu and ContextMenuItem lacks an implementation | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Holger Freyther <zecke> | ||||||||||||
| Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | christian, lars, lethalman88 | ||||||||||||
| Priority: | P2 | Keywords: | Gtk | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | Mac | ||||||||||||||
| OS: | OS X 10.4 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Holger Freyther
2007-11-24 04:47:38 PST
Created attachment 17507 [details]
ContextMenu Implementation WIP
Known issues or not yet checked:
- Is the ref counting done right? Should we sink it? Should we just ref it?
- We exploit the GObject to attach ContextMenuAction to a GtkMenuItem. We could keep them mapping in a hash map but would we store it in ContextMenu or ContextMenuItem.
- How to map that into an API?
- The code to activate the context menu by keyboard (in webkitgtkpage.cpp) is mostly taken from the Windows port. It would be nice to share some of the calculations to find the right element.
- We need to morph a GtkMenuItem to a GtkCheckMenuItem on the fly. So it might make sense to not create a GtkMenuItem in ContextMenuItem until we need it or not at all. With the internal API we do not know what we are going to be.
Anyway with this patch you can right click and see a context menu.
Nice work, apparently you were faster than me with this one. :) > - How to map that into an API? The "populate-popup" seems to be a natural choice. It provides a pointer to a GtkMenu to append or prepend custom items. This of course only really makes sense once we have means of retrieving the element in question. We probably want imageMenuItem's and make the edit items match those of a GtkEntry. And connect to "popup-menu" to show the menu via keyboard. > - We need to morph a GtkMenuItem to a GtkCheckMenuItem on the fly. So it might make sense to not create a GtkMenuItem in ContextMenuItem until we need it or not at all. With the internal API we do not know what we are going to be. On the fly creation of the items makes the most sense to me. Created attachment 18153 [details]
updated patch
I just updated the patch for the new API.
Comment on attachment 18153 [details]
updated patch
This didn't build when applied to SVN TOT. WebKit/gtk/WebCoreSupport/ContextMenuClientGtk.cpp needed some modifications to make it build.
After the modifications, I wasn't able to get a menu up. Is this patch meant to get that far?
Not really, it doesn't handle the mouse button. So i was just finding out a cleaner way of doing things. The purpose of this patch was to port the previous patch to the new API for now. Created attachment 18186 [details]
done port of context menu
I think the port from the original patch is done.
The menu shows but it's really really buggy, the problem is might be caused by the wrong use of ref/unref on items.
Created attachment 18187 [details]
base of context menu
Sorry was my mistake.
Comment on attachment 18187 [details]
base of context menu
r=me
ChangeLog entry next time please.
Will fix up one or two minor coding style issues eg. event.button()+1 and also the old terminology "gtkPage"
Landed in r29080. There are a few FIXMEs and TODOs we'll have to look into. Created attachment 18240 [details]
Create the GtkMenuItem on the fly.
This patch creates the GtkMenuItem on the fly. This allows to change the type, title and action easily. These values can be populated from an existing GtkMenuItem, e.g. when an item has been activated.
ContextMenuItem::setChecked is a pretty bad for the Gtk+ platform. We must know if an item is supposed to be checkable or not as we have GtkMenuItem and GtkCheckMenuItem and both get drawn differently. We should add another ActionType for checkable menuitems and make use of them.
Do you plan to also implement support for icons for appropriate menu items? The menus feel very alienated like this and icons would make a big difference. If you prefer I can just as well defer this to a new bug. Comment on attachment 18240 [details]
Create the GtkMenuItem on the fly.
r=me
|