Bug 75645

Summary: WebPlugins can't add items to context menus.
Product: WebKit Reporter: Bill Budge <bbudge>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: fishd, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Proposed Patch
none
Proposed Patch
none
Proposed Patch
fishd: review-
Proposed Patch none

Description Bill Budge 2012-01-05 12:56:33 PST
It would improve the user experience if there was a way for plugins to add items to context menus. In particular, the PDF viewer needs to allow users to rotate the page 90 degrees in certain cases, and it is awkward to keep adding icons for each operation.
Comment 1 Bill Budge 2012-01-07 10:53:20 PST
Created attachment 121555 [details]
Proposed Patch

This change adds the concept of rotation to media. The motivation is to allow the PDF viewer to add context menu commands to the existing page commands to support rotation of the pages.
Comment 2 Bill Budge 2012-01-07 10:54:37 PST
I need to upload another patch. There is an extraneous 'merge_custom_items' field that should be removed.
Comment 3 WebKit Review Bot 2012-01-07 10:55:07 PST
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment 4 Bill Budge 2012-01-07 11:53:01 PST
Created attachment 121556 [details]
Proposed Patch
Comment 5 Bill Budge 2012-01-07 11:56:37 PST
Created attachment 121557 [details]
Proposed Patch
Comment 6 Darin Fisher (:fishd, Google) 2012-01-09 08:37:49 PST
Comment on attachment 121557 [details]
Proposed Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=121557&action=review

> Source/WebKit/chromium/public/WebMediaPlayerAction.h:43
> +        Rotate90CW,

When we spoke about this design in person, I hadn't considered the fact
that this enum would also grow.  This one is very specifically about the
media player.  That's problematic since the media player doesn't support
rotation.  Same goes for WebView::performMediaPlayerAction.  It doesn't
seem correct to graft plugin-related command handling onto that.

Since it seems OK to refer to plugins as media, maybe the answer is to just
use more generic names for WebMediaPlayerAction and performMediaPlayerAction?
WebMediaAction and performMediaAction?
Comment 7 Bill Budge 2012-01-09 09:02:28 PST
In a previous version of this patch, I added a plugin specific enum and action. It requires all of that to be plumbed through on the Chromium side though. If it's OK with you, I will attempt to make the existing ones more generic as you suggest.
Comment 8 Darin Fisher (:fishd, Google) 2012-01-09 10:13:32 PST
(In reply to comment #7)
> In a previous version of this patch, I added a plugin specific enum and action. It requires all of that to be plumbed through on the Chromium side though. If it's OK with you, I will attempt to make the existing ones more generic as you suggest.

With the plugin specific enum, was there no entry point back into WebKit to handle the commands when they are selected?
Comment 9 Bill Budge 2012-01-09 10:28:44 PST
I had to add a method, performPluginAction, to WebView.h. So I am going to modify this patch, adding a new enum. If I understand you correctly, we should leave media player stuff alone. I think it makes the most sense to add a WebPluginAction struct, and a performPluginAction method, as this is all about plugins. This will require some extra code on the Chromium side but it's trivial, and the clarity on the WebKit side makes it worth it.

(In reply to comment #8)
> (In reply to comment #7)
> > In a previous version of this patch, I added a plugin specific enum and action. It requires all of that to be plumbed through on the Chromium side though. If it's OK with you, I will attempt to make the existing ones more generic as you suggest.
> 
> With the plugin specific enum, was there no entry point back into WebKit to handle the commands when they are selected?
Comment 10 Darin Fisher (:fishd, Google) 2012-01-09 11:28:26 PST
OK
Comment 11 Bill Budge 2012-01-09 13:23:37 PST
Created attachment 121719 [details]
Proposed Patch
Comment 12 WebKit Review Bot 2012-01-10 15:02:09 PST
Comment on attachment 121719 [details]
Proposed Patch

Clearing flags on attachment: 121719

Committed r104634: <http://trac.webkit.org/changeset/104634>
Comment 13 WebKit Review Bot 2012-01-10 15:02:13 PST
All reviewed patches have been landed.  Closing bug.