420148 - Flash context menu difficult to use with certain Flash movies/clips

Status: RESOLVED FIXED

(Core :: General, defect, P2)

Description

[Martijn Wargers (dead)]

See also the comment I made in bug 389931, comment 36:
"
Ok, with the latest patch, the cpu peg seems to be gone.
I noticed that right-clicking on a flash plugin, the flash animation now
continues, instead that it freezes. That's probably an improvement, I guess.
Except, it makes it sometimes difficult to click on one of the context
menu-items while the animation is playing.
For example: http://www.mooves.nl/ , choose "animatie" and choose one of the
clips, then right-click on one of the clips and zoom in/zoom out. Sometimes, it
doesn't respond to that. But I guess it's probably not related to the patch (or
only sideways).
"

It seems not happening that easily in regular builds, but I can still see it.
I see this especially with http://www.movenetworks.com/
Open that site in 2 tabs and it seems like with right-clicking, the Flash context menu has trouble showing up and clicking on a menu-item seems almost impossible.
I guess this has something to do with some Flash movies/clips filling up the message queue or something like that.

I don't know if this is supposed to be a Flash issue, but at least this started happening when something changed in Mozilla, at least.

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase - using Flash

In this case, when right-clicking and then dismissing the flash, Mozilla hangs.

Comment 2

[Martijn Wargers (dead)]

Attachment: testcase2 - using RealPlayer

When clicking on the 'i' in the RealPlayer plugin, Mozilla crashes withing 100ms after that action.

Comment 3

[Mats Palmgren (inactive)]

I filed bug 420884 for testcase 1 (unrelated to bug 389931) and bug 420886
for testcase 2 which I think is a different issue from this bug.

Comment 4

[Mats Palmgren (inactive)]

(In reply to comment #0)
> I guess this has something to do with some Flash movies/clips filling up the
> message queue or something like that.

Yeah, that's my impression too, possibly also (CPU) load related.
I can reproduce the problem quite easily at http://www.aftonbladet.se/
which is a Swedish tabloid site which usually has 7-8 big animated
Flash ads running and causes quite heavy CPU load.

I can't reproduce the problem at http://www.movenetworks.com/ which
currently shows a "Download Move Player" ad.
The context menu on animations at http://www.mooves.nl/ also works
reasonably well, but fails occasionally.

Comment 5

[Mats Palmgren (inactive)]

Attachment: wip2

I think I've found a solution for this.  The idea is to stop processing
native events from the appshell while we're processing gecko events from
NativeEventCallback() and instead try to return to the nested native
loop ASAP.

The Flash context menu works reliably for me with this patch and it also
seems to fix bug 421214.  AFAICT, it does not regress bug 389931.
Builds for testing are available here:
https://build.mozilla.org/tryserver-builds/2008-03-10_11:[e-mail]-420148_wip2/

Comment 6

[Steve England [:stevee]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008031012 Minefield/3.0b5pre ID:2008031012

The 2008-03-10_11:[e-mail]-420148_wip2 works nicely for me. No more getting two menus when right-clicking on something Flash, and when I do right click on Flash its context menu is responsive and it's easy to navigate through it. It doesn't seem to regress bug 389931 either, but I didn't test that much.

Comment 7

[Damon Sicore (:damons)]

+'ing w/ P2.  

Comment 8

[Dan Weiss]

I just tried out the fixed version linked above.  It works well most of the time, but I'm experiencing the bug on http://channel.nationalgeographic.com/channel/aftermath/
The first page's flash handles well, but if you open the popup link, then the browser goes slow and unresponsive as long as the new window is open.

It's tough, but I did managed to somehow close that popup by minimzing all windows from the windows taskbar, then right clicking to close windows.

Comment 9

[Mats Palmgren (inactive)]

(In reply to comment #8)
I see no difference between 3.0b1, 3.0b4 or my test build on this URL,
so it's an unrelated problem.  FWIW, the popup animation runs quite
slow in IE7 as well, although better than in Firefox.

Comment 10

[Mats Palmgren (inactive)]

Attachment: wip3

This is the same as wip2, except I added a OnDispatchedEvent() call
if we need to unblock (rare) just to make sure there is one in case
it was blocked earlier.
I think we should take this not just for the context menu bug, but it
also avoids creating unnecessary event loop nesting (which might help
making bug 420886 less likely to occur). Wip3 builds are here:
https://build.mozilla.org/tryserver-builds/2008-03-12_21:[e-mail]-420148_wip3/

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Need documentation for what mBlockNativeEvent really means, where it's declared.

+  if (mEventloopNestingState == eEventloopOther) {
+    mBlockNativeEvent = prevBlockNativeEvent;
+  }

Could we make the save/restore of mBlockNativeEvent unconditional? Saves code.

Other than that, looks good!

We really need an essay somewhere explaining how this stuff works.

Comment 13

[Mats Palmgren (inactive)]

Attachment: wip3 with nits fixed (checked in)

Comment 14

[Mats Palmgren (inactive)]

mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp 	1.9
mozilla/widget/src/xpwidgets/nsBaseAppShell.h 	1.9 

-> FIXED

Comment 16

[Dan Weiss]

Please re-open this bug.  Minefield and Firefox RC2 still have problems with context menus in situations where there is 100% CPU usage, such as full screen YouTube.
Open a context menu in Fullscreen YouTube, and Firefox becomes nearly unresponsive to anything.  Normally need to kill the process.  Firefox 2 never had this problem.

Comment 17

[Mats Palmgren (inactive)]

Dan, please file it as a separate bug and CC me.  Don't forget to
mention which plugin version you're using.  Thanks.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

And please point out the bug # here, so everyone who is interested in, can have a look at. Thanks.