Bug 75645 – WebPlugins can't add items to context menus.

• Component: Platform
• Status: RESOLVED FIXED

[Bill Budge]

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.

[Bill Budge]

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.

[Bill Budge]

I need to upload another patch. There is an extraneous 'merge_custom_items' field that should be removed.

[WebKit Review Bot]

Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

[Bill Budge]

Created attachment 121556 [details]
Proposed Patch

[Bill Budge]

Created attachment 121557 [details]
Proposed Patch

[Darin Fisher (:fishd, Google)]

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?

[Bill Budge]

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.

[Darin Fisher (:fishd, Google)]

(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?

[Bill Budge]

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?

[Darin Fisher (:fishd, Google)]

OK

[Bill Budge]

Created attachment 121719 [details]
Proposed Patch

[WebKit Review Bot]

Comment on attachment 121719 [details]
Proposed Patch

Clearing flags on attachment: 121719

Committed r104634: <http://trac.webkit.org/changeset/104634>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.