RESOLVED FIXED151217
ContextMenuController::contextMenuItemSelected only needs the action and title, not the full item 
https://bugs.webkit.org/show_bug.cgi?id=151217
Summary ContextMenuController::contextMenuItemSelected only needs the action and titl...
Anders Carlsson
Reported 2015-11-12 13:10:25 PST
ContextMenuController::contextMenuItemSelected only needs the action and title, not the full item
Attachments
Patch (10.96 KB, patch)
2015-11-12 13:12 PST, Anders Carlsson 		joepeck: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Anders Carlsson
Comment 1 2015-11-12 13:12:03 PST
Created attachment 265426 [details] Patch
Joseph Pecoraro
Comment 2 2015-11-12 13:21:16 PST
Comment on attachment 265426 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265426&action=review r=me > Source/WebKit2/ChangeLog:4 > + ContextMenuController::contextMenuItemSelected only needs the action and title, not the full item > + https://bugs.webkit.org/show_bug.cgi?id=151217 It was not clear to me that this approach is actually better. But you mention on IRC that by requiring a ContextMenuItem we often have to create a fake ContextMenuItem with fake values for its other state (like WebKit2), and that sucks. It would be nice to include something to this effect in a ChangeLog like this WebKit2 one where the fake object creation is most obvious. > Source/WebCore/page/ContextMenuController.h:32 > +#include "ContextMenuItem.h" You need this for the ContextMenuAction type? If you do need this include, you can drop the ContextMenuItem forward declaration. > Source/WebCore/page/ContextMenuProvider.h:34 > +#include "ContextMenuItem.h" Ditto, you can drop a forward declaration (or two).
Anders Carlsson
Comment 3 2015-11-12 13:22:25 PST
Committed r192381: <http://trac.webkit.org/changeset/192381>
Note You need to log in before you can comment on or make changes to this bug.