[Webkit-unassigned] [Bug 67660] [GTK] Add a way to expose well known names for items in the default context menu
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Sep 7 00:13:33 PDT 2011
https://bugs.webkit.org/show_bug.cgi?id=67660
--- Comment #3 from Carlos Garcia Campos <cgarcia at igalia.com> 2011-09-07 00:13:33 PST ---
(In reply to comment #2)
> (From update of attachment 106450 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=106450&action=review
>
> I like this patch a lot. I think it has the potential to remove a *ton* of code from Epiphany and other WebKitGTK+ clients. I'm giving the first r+ here, but we need one more to approve the new API. Also, I think this same approach should be used for WebKit2.
>
> > Source/WebKit/gtk/ChangeLog:3
> > + Add webkit_context_menu_item_get_action
>
> I think this one is extra...it should be part of the description below.
Yes, it's the git commit message that prepare-Changelog leaves there, and I forgot to remove it :-P
> > Source/WebKit/gtk/WebCoreSupport/ContextMenuClientGtk.cpp:113
> > + g_object_set_data(G_OBJECT(bidiMenuItem.gtkAction()), "gtk-unicode-menu-entry", (gpointer)&bidi_menu_entries[i]);
>
> I'm almost certain you shouldn't need to cast to gpointer here. If you do though, you should use C++ style casts.
This code was already there, I'll check if we can avoid the cast or use a C++ style cast.
> > Source/WebKit/gtk/webkit/webkitglobals.cpp:306
> > + * can be used to know the items present in the default context menu.
>
> know -> determine
Ok.
> > Source/WebKit/gtk/webkit/webkitglobals.cpp:308
> > + * WebKitWebView:context-menu signal:
>
> The second : should be a '.'
Ok.
> > Source/WebKit/gtk/webkit/webkitglobals.cpp:340
> > + * GList* items = gtk_container_get_children (GTK_CONTAINER (default_menu));
> > + * GList* l;
> > + * GtkAction *action;
> > + *
> > + * for (l = items; l; l = g_list_next (l)) {
> > + * GtkMenuItem* item = (GtkMenuItem *)l->data;
> > + *
> > + * if (GTK_IS_SEPARATOR_MENU_ITEM (item)) {
> > + * /* It's separator, do nothing */
> > + * continue;
> > + * }
> > + *
> > + * switch (webkit_context_menu_item_get_action (item)) {
> > + * case WEBKIT_CONTEXT_MENU_ACTION_NO_ACTION:
> > + * /* No action for this item */
> > + * break;
> > + * /* Don't allow to ope links from context menu */
> > + * case WEBKIT_CONTEXT_MENU_ACTION_OPEN_LINK:
> > + * case WEBKIT_CONTEXT_MENU_ACTION_OPEN_LINK_IN_NEW_WINDOW:
> > + * action = gtk_activatable_get_related_action (GTK_ACTIVATABLE (item));
> > + * gtk_action_set_sensitive (action, FALSE);
> > + * break;
>
> Awesome documentation.
>
> > Source/WebKit/gtk/webkit/webkitglobals.cpp:346
> > + * /* Do somethig with submenu */
>
> Hrm. Maybe expand this comment. Can the embedder crawl this menu too?
Right, I'll add an example of how to add a new menu item.
> > Source/WebKit/gtk/webkit/webkitglobals.cpp:362
> > +WebKitContextMenuAction webkit_context_menu_item_get_action (GtkMenuItem* item)
>
> Extra space after webkit_context_menu_item_get_action.
Ok.
Thanks!
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list