Created attachment 193364 [details] WIP patch (missing Changelog) GTK could switch to cross platform context menu implementation and reduce Context Menu related code significantly.
Comment on attachment 193364 [details] WIP patch (missing Changelog) View in context: https://bugs.webkit.org/attachment.cgi?id=193364&action=review > Source/WebCore/platform/gtk/ContextMenuItemGtk.cpp:146 > + //if (!gtk_action_get_accel_path(action)) After this patch, on MiniBrowser (WK2) I can see the Context Menu items being rendered properly but when I click on them nothing happens. If I uncomment this early return, though, the check for gtk_action_get_accel_path(action) always fail. Thus, early returning here. I couldn't find what could have broken this "accel_path", since I couldn't find the analog gtk setter function (gtk_action_SET_accel_path) being called anywhere...
Comment on attachment 193364 [details] WIP patch (missing Changelog) View in context: https://bugs.webkit.org/attachment.cgi?id=193364&action=review >> Source/WebCore/platform/gtk/ContextMenuItemGtk.cpp:146 >> + //if (!gtk_action_get_accel_path(action)) > > After this patch, on MiniBrowser (WK2) I can see the Context Menu items being rendered properly but when I click on them nothing happens. If I uncomment this early return, though, the check for gtk_action_get_accel_path(action) always fail. Thus, early returning here. I couldn't find what could have broken this "accel_path", since I couldn't find the analog gtk setter function (gtk_action_SET_accel_path) being called anywhere... I have a theory: I believe in the previous implementation the GtkMenuItem object would have been created when you get here, and would have caused the action to get an accel path set.
(In reply to comment #2) > I have a theory: I believe in the previous implementation the GtkMenuItem object would have been created when you get here, and would have caused the action to get an accel path set. But it would have had its accel path set, right? I can't find any call to gtk_action_set_accel_path(), assuming this was the right function to be called... (I have close to 0 experience with GTK =/)
Right, my theory is the creation of a widget (like the menu item that gets created without this patch) attached to the action causes the accel path to be set as a side-effect, that means you won't find a set call inside WebKit, it's being done inside GTK+.
I have put some more investigation into this. The problem is that after this patch we are not keeping a GtkMenu* anywhere anymore and we should create it on demand. WebKitGTK's design relys too much in WebCore creating and keeping this pointer inside ContextMenu. The same goes to ContextMenuItem and GtkMenuItem*. I did have some progress there, but I've reached a point that my lack of time to understand Gtk and GObject specificities prevent me of going on. At least not without risking WebKitGTK+'s API implementation. =/ It would be better if someone from the GTK port could look into this when he/she have enough free time for it. Thanks for helping!