| Summary: | ContextMenuController::contextMenuItemSelected only needs the action and title, not the full item | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Anders Carlsson <andersca> | ||||
| Component: | New Bugs | Assignee: | Anders Carlsson <andersca> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | ||||||
| Priority: | P2 | ||||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Anders Carlsson
2015-11-12 13:10:25 PST
Created attachment 265426 [details]
Patch
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). Committed r192381: <http://trac.webkit.org/changeset/192381> |