595132 - pausing Flash Video (e.g. YouTube) prevents Opening of Toolbar context Menu ,Tab context Menu

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Rafael W]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100909 Firefox/4.0b6pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100909 Firefox/4.0b6pre

then i watch a youtube video i can't make a right mouse klick in the house buttom

Reproducible: Always

Steps to Reproduce:
1.open youtube.com
2.watch a video on youtube.com
3.right mouse klick on the "house"-buttom
Actual Results:  
nothing

Expected Results:  
a little menu must open, then i make a right mouse klick on the buttom

Comment 1

[(mostly gone) XtC4UaLL [:xtc4uall]]

Ermm, what is the "House"-Button?
Maybe you can provide and attach a Screenshot?
Also, what Version of the Flash Plugin are you using (check about:plugins)?

Is your Issue reproduceable Extension-less in Safe-Mode (where Plugins do work) and/or using a new Profile?
https://support.mozilla.com/en-US/kb/Safe+Mode
https://support.mozilla.com/en-US/kb/Managing+profiles#Creating_a_profile

What happens if you disable Hardware Acceleration in Tools/Options/Advanced/General?
What happens if you disable "Out of Process Plugins (OOPP)" via about:config and setting "dom.ipc.plugins.enabled" and/or "dom.ipc.plugins.enabled.npswf32.dll" to false?

Comment 2

[Rafael W]

I mean this Buttom.(http://p5.p1x.de/user/mini/2b/a7/d78ca6ed-39828780.png)
I'm using the version: 10.1.82.76

If I have all my add-ons in safe mode disabled, this problem come anyway.
If i disable Hardware Acceleration,too - the bug remains

Always when I play a youtube video and make a right mouse klick on the button (picture),the little menu is displayed
But if I press the stop-buttom in the video ,the little menu don't appear after a right mouse klick.

Comment 3

[(mostly gone) XtC4UaLL [:xtc4uall]]

Ahhh! Now i see this too.

So the Step to Reproduce this are:
* open a Flash Video and let it play
* pause it using the Pause Button
* do a Right-Click on any Toolbar (does not have to be a Button Item)

expected:
Toolbar Customization Menu appears

actual:
no Menu appears

A quick Check on a Regression Range reveals that this regressed between Beta 4 <-> 5.

Comment 4

[(mostly gone) XtC4UaLL [:xtc4uall]]

Ok, further Digging reveals that this broke within
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e1d55bbd1d1d&tochange=404b79632ff4

Checkins for Bug 130078, Bug 590468, Bug 587944 sound suspicious.

Comment 5

[Timothy Nikkel (:tnikkel)]

So whenever the plugin has focus the chrome buttons (back/forward/home) don't get right click events somehow. They get left click and hover, but not right click. Doesn't seem to happen on Linux. Right clicking on other things, like the location bar or outside the plugin but inside the content area work as expected.

Comment 7

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Regression pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=01fa971e62ee&tochange=0886ad6e6aaa

Build from 7804b5cf4313: fails:
Build from 4722b491cd0d : works:
Landing of Bug 130078 causes this.

According to Alice0775 White

Comment 8

[Timothy Nikkel (:tnikkel)]

Bug 604338 which was marked as a duplicate of this had a blocking2.0 nom, transferring to here.

Comment 12

[Jim Mathies [:jimm]]

Attachment: patch

This wasn't an issue until we removed tab widgets, since the top level window plugin focus wasn't affected by content ime state.

Comment 13

[Jim Mathies [:jimm]]

note, I'm working up a test for this as well.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #12)
> Created attachment 494185 [details] [diff] [review]
> patch
> 
> This wasn't an issue until we removed tab widgets, since the top level window
> plugin focus wasn't affected by content ime state.

How about windowless plugin? This patch looks preventing context menu on windowless plugin. I think that we need to check:

1. whether the context menu message is caused by mouse event or keyboard event.
2. if it's caused by mouse event, we need to check whether the point is on a plugin.

But I don't have simple idea about 2 to fix this bug in widget level.

# This bug is similar to bug 404297.

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Um... The lParam value isn't useable for checking the position because it doesn't work fine on secondary display because negative value may be but the type is unsigned.

Comment 16

[Jim Mathies [:jimm]]

(In reply to comment #14)
> (In reply to comment #12)
> > Created attachment 494185 [details] [diff] [review] [details]
> > patch
> > 
> > This wasn't an issue until we removed tab widgets, since the top level window
> > plugin focus wasn't affected by content ime state.
> 
> How about windowless plugin? This patch looks preventing context menu on
> windowless plugin. I think that we need to check:
> 
> 1. whether the context menu message is caused by mouse event or keyboard event.
> 2. if it's caused by mouse event, we need to check whether the point is on a
> plugin.
> 
> But I don't have simple idea about 2 to fix this bug in widget level.
> 
> # This bug is similar to bug 404297.

Yes, you're right. I was tricked by flash, they use the mouse down event to display the context. Silverlight is broken with this patch. I'll roll a new fix.

Comment 17

[Jim Mathies [:jimm]]

Attachment: tests

Comment 18

[Jim Mathies [:jimm]]

(In reply to comment #16)
> (In reply to comment #14)
> 
> Yes, you're right. I was tricked by flash, they use the mouse down event to
> display the context. Silverlight is broken with this patch. I'll roll a new
> fix.

Actually I was wrong about that. I was testing with ipc disabled, and mouse events are totally broken with sl with non-ipc.

It doesn't look like we deliver wm_contextmenu to windowless plugins, so I think this fix is still ok.

Comment 19

[Jim Mathies [:jimm]]

So basically we don't deliver this to plugins at all. If the context menu event is received it's dropped for windowless. For windowed, plugins get their own context menu event when the mouse interaction takes place on their window. This even is targeted at our UI, and we should process it as such.

The simple fix here is to remove the |case WM_CONTEXTMENU| in ProcessMessageForPlugin, it's not needed.

Comment 20

[Jim Mathies [:jimm]]

Attachment: patch

Don't trap and send context menu events destined for the browser to plugins.

Comment 21

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 494304 [details] [diff] [review]
patch

okay, but if some plugins doesn't work fine with this patch, we should backout it. But probably, there are no such plugins.

Comment 22

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> The simple fix here is to remove the |case WM_CONTEXTMENU| in
> ProcessMessageForPlugin, it's not needed.

No, it's needed. The plugin event just sends the native UI messages to plugins rather than other our events. But probably, plugins are checking right click event on its window. So, I guess that the patch doesn't make any regression actually.

Comment 23

[Jim Mathies [:jimm]]

(In reply to comment #22)
> > The simple fix here is to remove the |case WM_CONTEXTMENU| in
> > ProcessMessageForPlugin, it's not needed.
> 
> No, it's needed. The plugin event just sends the native UI messages to plugins
> rather than other our events. 


Hmm, but they aren't, windowed or windowless. You can test this yourself by putting a break point on PluginInstanceParent's NPP_HandleEvent after breaking on WM_CONTEXTMENU in ProcessMessageForPlugin. The event never reaches the plugin. Maybe this is a different bug? Regardless plugins aren't expecting WM_CONTEXTMENU currently so there's no point in processing it.

Comment 24

[Jim Mathies [:jimm]]

I'll file off another bug on events WM_DEADCHAR -> WM_UNDO. Those currently aren't delivered. We can figure out if we want to start delivering them or simply remove the code. I think though that for the release we need to fix the context menu thing for the browser. The plugin event delivery issue isn't a priority.

Comment 25

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Ah, the events are only sent to plugins if it's a windowless flash player.
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsObjectFrame.cpp#438

We should send native events for all plugins. However, we have limited it because we decided it in feature frozen time of our previous release. So, we should do it next alpha cycle.

So, if windowless flash player doesn't use WM_CONTEXTMENU, there is no regression.

Comment 26

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 494294 [details] [diff] [review]
tests

r=masayuki.

NOTE:

> +        onload="onLoad();"

I'm not sure whether this is okay. If you will see random orange about focus problem, you should use:

window.opener.wrappedJSObject.SimpleTest.waitForFocus(runTest, window);

Comment 27

[Jim Mathies [:jimm]]

(In reply to comment #26)
> Comment on attachment 494294 [details] [diff] [review]
> tests
> 
> r=masayuki.
> 
> NOTE:
> 
> > +        onload="onLoad();"
> 
> I'm not sure whether this is okay. If you will see random orange about focus
> problem, you should use:
> 
> window.opener.wrappedJSObject.SimpleTest.waitForFocus(runTest, window);

updated. Sent this to try for test runs.

Comment 28

[Jim Mathies [:jimm]]

http://hg.mozilla.org/mozilla-central/rev/cb69605b4028
http://hg.mozilla.org/mozilla-central/rev/8bd4943447fb