453617 - right-click on windowless plugins shows Firefox context menu

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Benjamin Otte]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en; rv:1.9.0.1) Gecko/20080528 Epiphany/2.22 Firefox/3.0
Build Identifier: 

Right-clicking on a windowless plugin causes the right click to be handled by both Mozilla and the plugin in certain circumstances, while it should only be handled by the plugin.

Reproducible: Always

Steps to Reproduce:
1) set dom.event.contextmenu.enabled to false in your configuration
2) install swfdec-mozilla 0.7.x or newer (likely any other windowless plugin such as Adobe's Flash 10 beta will also work)
3) open http://www.comedycentral.com/shows/the_colbert_report/index.jhtml (most other sites don't exhibit this problem)
4) right-click on the big Flash in the centre (likely a huge gray rectangle with a play button)
Actual Results:  
Two context menus pop up: The one from Flash is below Mozilla's

Expected Results:  
Only the Flash context menu shows up.

This had been filed as https://bugs.launchpad.net/ubuntu/+source/ubufox/+bug/242223 and also contains a screenshot of the problem.

I think this bug is Linux-specific, as I've never noted any similar issues on Windows (I'm having that preference set to false there).

I'm setting this to major because there's a GUI to change this preference (Content / Advanced Javascript settings / Disable or replace context menus)

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

Attachment: Patch v1.0

We should not recover the context menu event on plug-ins even if "dom.event.contextmenu.enabled" is false.

Comment 2

[Hideo Oshima]

(In reply to comment #1)
> Created an attachment (id=344801) [details]
> Patch v1.0
> 
> We should not recover the context menu event on plug-ins even if
> "dom.event.contextmenu.enabled" is false.

I tried this patch.
It seems good for me.
Thanks.

Comment 3

[Neil Deakin]

Comment on attachment 344801 [details] [diff] [review]
Patch v1.0

>+          type == nsIObjectLoadingContent::TYPE_PLUGIN) {
>+        eventEnabled = PR_TRUE;
>+      }

It would be clearer if this just returned directly instead of setting eventEnabled.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

Attachment: Patch v1.1

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

Attachment: Patch v1.2

oops, this is better.

Comment 6

[[e-mail]]

Comment on attachment 344900 [details] [diff] [review]
Patch v1.2

>+          type == nsIObjectLoadingContent::TYPE_PLUGIN) {
This is only true for a genuine plugin, right? Is it worth removing the nsPluginDOMContextMenuListener in nsObjectFrame.cpp and just using code here to block context menus on plugins? Hmm, I guess that doesn't work for embedders.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

(In reply to comment #6)
> (From update of attachment 344900 [details] [diff] [review])
> >+          type == nsIObjectLoadingContent::TYPE_PLUGIN) {
> This is only true for a genuine plugin, right? Is it worth removing the
> nsPluginDOMContextMenuListener in nsObjectFrame.cpp and just using code here to
> block context menus on plugins? Hmm, I guess that doesn't work for embedders.

landed the patch. I'll check your suggestion after I go back to my town.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

oops, sorry for the spam.