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

RESOLVED FIXED in mozilla1.9beta5

Status

()

Core
General
P2
normal
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Martijn Wargers (zombie), Assigned: mats)

Tracking

({regression})

Trunk
mozilla1.9beta5
x86
Windows XP
regression
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

10 years ago
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.
(Reporter)

Comment 1

10 years ago
Created attachment 307220 [details]
testcase - using Flash

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

Comment 2

10 years ago
Created attachment 307222 [details]
testcase2 - using RealPlayer

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

Comment 3

10 years ago
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.
(Assignee)

Comment 4

10 years ago
(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.

Updated

10 years ago
Flags: blocking1.9?
(Assignee)

Comment 5

10 years ago
Created attachment 308494 [details] [diff] [review]
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:05-mpalmgren@mozilla.com-420148_wip2/
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:05-mpalmgren@mozilla.com-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.
+'ing w/ P2.  
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2

Comment 8

10 years ago
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.
(Assignee)

Comment 9

10 years ago
(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.
(Assignee)

Comment 10

10 years ago
Created attachment 309148 [details] [diff] [review]
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:46-mpalmgren@mozilla.com-420148_wip3/
Assignee: nobody → mats.palmgren
Attachment #307220 - Attachment is obsolete: true
Attachment #307222 - Attachment is obsolete: true
Attachment #308494 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #309148 - Flags: superreview?(roc)
Attachment #309148 - Flags: review?(roc)
(Assignee)

Updated

10 years ago
Blocks: 421214
(Assignee)

Updated

10 years ago
Duplicate of this bug: 422710
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.
Attachment #309148 - Flags: superreview?(roc)
Attachment #309148 - Flags: superreview+
Attachment #309148 - Flags: review?(roc)
Attachment #309148 - Flags: review+
(Assignee)

Comment 13

10 years ago
Created attachment 309573 [details] [diff] [review]
wip3 with nits fixed (checked in)
(Assignee)

Comment 14

10 years ago
mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp 	1.9
mozilla/widget/src/xpwidgets/nsBaseAppShell.h 	1.9 

-> FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta5
No longer blocks: 419668
(Assignee)

Updated

10 years ago
Duplicate of this bug: 424315

Comment 16

10 years ago
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.
(Assignee)

Comment 17

10 years ago
Dan, please file it as a separate bug and CC me.  Don't forget to
mention which plugin version you're using.  Thanks.
And please point out the bug # here, so everyone who is interested in, can have a look at. Thanks.
You need to log in before you can comment on or make changes to this bug.