Closed Bug 595132 Opened 14 years ago Closed 14 years ago

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

Categories

(Core :: General, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- final+

People

(Reporter: Rafael.Winkler93, Assigned: jimm)

References

(Blocks 1 open bug, )

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

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
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?
Version: unspecified → Trunk
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.
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.
Summary: can't make right mouse klick then opened a flash video → pausing Flash Video (e.g. YouTube) prevents Opening of Toolbar Customization Menu
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.
Product: Firefox → Core
QA Contact: general → general
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
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
Blocks: 130078
Bug 604338 which was marked as a duplicate of this had a blocking2.0 nom, transferring to here.
blocking2.0: --- → ?
Summary: pausing Flash Video (e.g. YouTube) prevents Opening of Toolbar Customization Menu → pausing Flash Video (e.g. YouTube) prevents Opening of Toolbar context Menu ,Tab context Menu
Assignee: nobody → jmathies
Blocks: 615725
Attached patch patch (obsolete) — Splinter Review
This wasn't an issue until we removed tab widgets, since the top level window plugin focus wasn't affected by content ime state.
Attachment #494185 - Flags: review?(masayuki)
note, I'm working up a test for this as well.
(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.
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.
(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.
Attachment #494185 - Attachment is obsolete: true
Attachment #494185 - Flags: review?(masayuki)
Attached patch testsSplinter Review
(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.
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.
Attached patch patchSplinter Review
Don't trap and send context menu events destined for the browser to plugins.
Attachment #494304 - Flags: review?(masayuki)
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.
Attachment #494304 - Flags: review?(masayuki) → review+
> 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.
(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.
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.
Attachment #494294 - Flags: review?(masayuki)
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 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);
Attachment #494294 - Flags: review?(masayuki) → review+
Blocks: 615894
(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.
http://hg.mozilla.org/mozilla-central/rev/cb69605b4028
http://hg.mozilla.org/mozilla-central/rev/8bd4943447fb
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
Flags: in-litmus?
Depends on: 798337
You need to log in before you can comment on or make changes to this bug.