Closed Bug 300057 Opened 19 years ago Closed 16 years ago

onmouseover=alert makes browser window unusable (modal event loop issues)

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

Product:
Core
Component:
DOM: UI Events & Focus Handling
Platform:
PowerPC
macOS
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED WORKSFORME

People

(Reporter: bugzilla, Unassigned)

References

Details

(Keywords: dataloss, regression, testcase)

Attachments

(6 files, 1 obsolete file)

Even better testcase (WARNING: BREAKS BROWSER)
(deleted), text/html
Details
Example of sampled stack
(deleted), text/plain
Details
Another example of sampled stack. slightly different
(deleted), text/plain
Details
stack from second alert taken from gdb
(deleted), text/plain
Details
More demanding test (MAY LOCKUP BROWSER)
(deleted), text/html
Details
WIP patch
(deleted), patch
Details | Diff | Splinter Review 
Build id: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b3)
Gecko/20050707 Firefox/1.0+

This bug is a spinoff from Bug 297561. It's more serious on mac though. 

Visit testcase, after you dismiss the alert box, the entire window will not
respond to any interaction. All open tabs in current window are not accessible
and might result in dataloss.

Note that you can open a new window (menues still work), but the current one is
"dead".

Steps to reproduce:
1. Go to attachment 186120 [details]
2. Hover yellow box
3. Dismiss dialog
Flags: blocking1.8b4?
Keywords: regression
Test 1. Using Tabs

I got a similar reaction, but it wasn't a 'show-stopper' when using tabs.
However i imagine it would be if this was an element on a page. I opened the
attachment in a tab and other tabs and toolbar elements were unresponsive when
individually clicking on them, however it is possible to bring up a new window
and continue browsing as normal; or alternatively, closing the attached example
tab. It should be noted that closing the example tab left artifacts in the
previous tab: The yellow "hover me, then dismiss the dialog" box remains and
there is some visual blockiness where the two tabs had been joined.

The dialog that pops up when i initially tested gave me an option to cancel, i
wasn't able to reproduce this dialog with the cancel option again.

Test 2. New Window

When opening the attached example in a new window the menu bar widgets and
bookmarks bar are totally unresponsive as José has reported. I know almost
nothing of java scripting, which unfortunately makes me unable to test whether
this example is in any way likely to be seen in the wild.
Addendum for comment #1: Apologies, OS X 10.4.2 Deer Park Alpha 2
Josh, we need your help on this one if you can.

Tracy, Zach, or Marcia can you guys help investigate this?
Assignee: events → joshmoz
Flags: blocking1.8b4? → blocking1.8b4+
Attached file Better testcase (obsolete) (deleted) — 
Here's a better testcase that doesn't have multiple <body> tags and leaves out
the css since it's not required for this to happen.
Heh, so this is what has been hosing me now and then on Mac.  I had no idea what
was causing it.  As previously stated, once you hit this bug, a few items may
work, but to have a fully functional browser you have to restart DP.  
Note that this applies whenever there is an element with an onmouseover=alert
property. Once the alert fires, the browser window is unusable. The element does
not nessecerially have to be a div, <p> and <img> will cause the same behavior.
Summary: Dismissing alert box makes current window unaccessible → onmouseover=alert makes browser window unusable
Sorry, that last testcase still has an error in it. This one actually closes
the html tag. It also causes the entire browser to lock up. The only way to
restore the browser is to force quit.
Attachment #190177 - Attachment is obsolete: true
Finding a regression window is not going to be very useful, as this was broken
in 1.0. See also bug 32415.
This is a problem with modal event loops. Using ThreadViewer, I can see that
sometimes we appear to fail to exit from the modal loop for the alert sheet, so
on the next mouseover we end up entering another modal loop. I'll attach some
stacks that show this.
Summary: onmouseover=alert makes browser window unusable → onmouseover=alert makes browser window unusable (modal event loop issues)
The problem here is that we still accept mouse move events in the content area
during a modal dialog for the window (see nsMacMessagePump::DispatchEvent).
Notice that the cursor changes over a link even with the sheet open. That
shouldn't happen either.
Even with a working modal event filter, we still have issues here because the
mac widget code hangs onto the "last wiget pointed to" in a global, so that it
can send it a last MOUSE_EXIT when you move over a different window. That
MOUSE_EXIT call is invoking mouseDown, and hence another nested event loop. Yuck.
This testcase has a refreshing <div> that makes things even worse.
Ugh. What a mess. With that last testcase, we recurse, making 2 appshells/modal
event loops each time. Some of those are happening because the view manager is
ending synthetic mouse events (from a plevent). We're not doing any plevent
filtering in our modal event loop, so we're probably hosed on multiple fronts.
Depends on: 301791
So, fixing this requires:
1. implementing nsWindow::ModalEventFilter() to filter out events not directed
   at the given window.
2. working around some stuff in mac widget that shoves the "last wiget hit" into
   a global (which can bypass the modal filter)
3. hardest of all, making sure that plevents are correctly filtered, which
   requires XP nsIEventQueue API changes.

All these are risky, and this stuff has never worked anyway. I'd suggest pushing
it off.
I recommend not blocking on this as well, for the reasons Simon just gave.
Flags: blocking1.8b4+ → blocking1.8b4?
Flags: blocking1.8b4? → blocking1.8b4-
I've implemented most of what is necesary to make nested plevent queues work on
Mac, but that last testcase still causes Mac to recursively throw up alerts.
What I don't understand is this:

PresShell::DidDoReflow() (here, I think, being called because of the animated
DIV in the parenet window) calls nsViewManager::SynthesizeMouseMove(), which
posts a plevent to the current event queue (which will be the modal alert's
event queue). That event gets processed, which tickles the "onmouseover" of the
div, which throws the alert again.

Why doesn't this happen on other platforms?
See bug 303484. If we made modal dialogs such as alert() and Save As() block
event handlers from running in the modally disabled window(s), then this would
be a lot better.

I'm not sure why your mouse event is recursively firing...
Depends on: 303484
(In reply to comment #18)
> PresShell::DidDoReflow() (here, I think, being called because of the animated
> DIV in the parenet window) calls nsViewManager::SynthesizeMouseMove(), which
> posts a plevent to the current event queue (which will be the modal alert's
> event queue). That event gets processed, which tickles the "onmouseover" of the
> div, which throws the alert again.
> 
> Why doesn't this happen on other platforms?

In that testcase, it does happen on other platforms.
Attached patch WIP patch (deleted) — — Splinter Review 
This patch shows my direction with this. It consists of several parts:

1. In nsWindow.cpp, implementing nsWindow::ModalEventFilter() to filter out
   events not destined for our window, and fixing some widget* globals in
   the mac widget code that allowed events to leak through to widgets in other
   windows.

2. adding a new nsIEventQueueRegistrarService API that toolkits implement to
   allow then to activate and deactivate nsIEventQueues.

3. calling into the nsIEventQueueRegistrarService from nsEventQueueService when

   we create, push and pop event queues.

4. extending the plevent API in two ways:
    i) (optional) adding a void* "user data" slot that clients can use to
       store whatever data they want. This is actually unsued in this patch,
       but could replace all the hash table crap that the unix platforms have
       in their appshell code.

   ii) adding fairly generic accessors for "native data" for platforms on
       which PL_GetEventQueueSelectFD() isn't enough.

5. Implementing nsEventQueueRegistrarService in Mac widget to add and remove
   CFRunLoopSources from the main runloop to activate/deactivate the
   associated queues.

With nsIEventQueueRegistrarService I've tried to codify the requirement that
for certain platforms event queueu modaility with plevent is not automatic. The
other non-windows platforms should migrate their appshell code into
implementations of nsIEventQueueRegistrarService, and then we can remove the
appshell spin up/spin down crap.
Could roc, dougt or someone look over the last patch and tell me if this is a
reasonable approach?
Is this targeted at 1.8 or 1.9?

For 1.9 I wonder whether we can get rid of multiple event queues.
This was punted on for 1.8 as being too risky, I think.
Then let's put this on ice for a while. We need to investigate whether having
multiple event queues per thread is actually necessary. If we could do away with
it, many problems would be solved, including this one I think.
*** Bug 318698 has been marked as a duplicate of this bug. ***
Elimination of event queues is bug 326273 (and being reviewed now).
Depends on: nsIThreadManager
Flags: blocking1.9?
DoS is not a blocker.
Flags: blocking1.9? → blocking1.9-
Assignee: joshmoz → nobody
Isn't this bug fixed? All testcases attached WFM using Firefox 3 beta 1. Using Tiger.
This bug has been WFM for a while. --> WORKSFORME
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → WORKSFORME
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: