Closed Bug 297561 Opened 19 years ago Closed 19 years ago

onmouseover, javascript alert shows twice


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

Windows XP
Not set





(Reporter: bugzilla, Assigned: emaijala+moz)



(Keywords: regression, testcase, verified1.8, Whiteboard: [no l10n impact])


(3 files, 3 obsolete files)

Build id: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2)
Gecko/20050613 Firefox/1.0+

Steps to reproduce:
1. load testcase
2. hover text, it should show an alert
3. Note that after pressing again, the alert comes up again

This works fine in Firefox 1.0.x, so it seems like a regression. Will try to
find out when this happened.
Attached file Testcase (obsolete) —
Martijn Wargers pointed out in the mozillazine forums that the event is
triggered only once, so it seems to be specific to alert dialogs
Attached file replacement testcase 1
Attachment #186111 - Attachment is obsolete: true
Summary: onmouseover showig alert shows it twice → onmouseover , javascript alert shows twice
Had fun looking in the archives and found that it works in:

but is broken in:

There are build in between to test, but not for windows that i use
Possible; there is weird focus-type stuff going on when modal windows are
involved, and it could be confusing the ESM....
Flags: blocking1.8b3?
doesn't seem to fail on Linux/GTK2. Can you confirm that this is Windows only?
(In reply to comment #7)
> doesn't seem to fail on Linux/GTK2. Can you confirm that this is Windows only?

Just tested this on Mac OS X 10.3.9, build 20050621. 

The alert is shown once, but once you dismiss it, the window will not respond
any more, hovering the div will not cause an alert to appear, pressing the
reload button does nothing etc. This is really a blocker issue on mac, should I
file a new bug for that or does this bug cover that as well? 
To clarify my last comment. Firefox doesn't freeze. The menues still work, so I
dont need to kill Firefox, if I open a new window via the file menu, it will
work just fine. But the window that the testcase was run on, has to be closed.
Flags: blocking1.8b4?
Flags: blocking1.8b3?
Flags: blocking1.8b3-
OK, this works on Linux/GTK1 too.  Ccing some win32 folks for help.
On Win32 a mouse exit event is posted by the mouse trailer right after the
dialog is displayed. nsEventStateManager notes the cursor is still inside the
view bounds and converts it to a mouse move event. This, I believe, triggers
another dialog. Maybe the mouse trailer needs tweaking..
Blocks: 291507
Blocks: 300057
(In reply to comment #10)
>OK, this works on Linux/GTK1 too.  Ccing some win32 folks for help.
Actually, on GTK1, if I hover the yellow div, and then move the alert to
partially overlap the yellow div, then mouse over the yellow div and then
directly over the alert then a further alert is triggered. Subsequent mousing
between the yellow div and the first alert opens two further alerts each time.
Whiteboard: [no l10n impact]
Hmm.. I tried disabling mouse trailer but for some reason everything went
haywire (dozens of dialogs...). Anyway, shouldn't we prevent onmouseover from
firing at all at least when a modal dialog is open?
(In reply to comment #13)
> Anyway, shouldn't we prevent onmouseover from firing at all at least when a
modal dialog is open?
Yes, at least, that is what IE6 is doing.
Attached patch patch (obsolete) — Splinter Review
It seems to me the alert window wasn't a modal dialog at all.
This doesn't seem to fix the bug, though.
Flags: blocking1.8b4? → blocking1.8b4+
Comment on attachment 189871 [details] [diff] [review]

johnny, can you review this for us?
Attachment #189871 - Flags: review?(jst)
Attached patch Win32 patch (obsolete) — Splinter Review
This is the best I can do without poking nsEventStateManager and others I'm
afraid to touch. This will just fake the position of the NS_MOUSE_EXIT message
sent by the mouse trailer so that nsEventStateManager won't convert it to
Attachment #190594 - Flags: review?(roc)
Assignee: events → emaijala
Can you tell me what widgets are involved here? In bug 297080 I made it so that
MOUSE_EXIT events from a toplevel window are always treated as exits.
I think the mouse trailer should only watch top-level windows ... if that was
done right, I think the patch in bug 297080 would make this unnecessary.
Well, I can try, but last time I tested just disabling the mouse trailer, it
went completely haywire. I've spent quite some time trying to  find the reason
for this behavior, but these are just a pain to debug..
Comment on attachment 190594 [details] [diff] [review]
Win32 patch

I think this should be done differently.
Attachment #190594 - Flags: review?(roc)
Basically, I think mCaptureWindow and mMouseHoldWindow should always be
top-level windows only. MouseTrailer can find the top-level window in
SetMouseTrailerWindow, and I guess mCaptureWindow wherever that gets set.
I tried this approach, but for some reason I couldn't quite get hold on the test
case starts creating dialogs like crazy when the mouse cursor is moved to the
first dialog after hovering the text. It stops when I manage to close all the
dialogs. At the same time this is displayed in the console:

WARNING: event queue chain length is 13. this is almost certainly a leak., file
c:/mozilla_source/mozilla/xpcom/threads/nsEventQueue.cpp, line 553

The chain length is incremented for every dialog created. roc, any ideas?
That error is related to 303484.

Maybe what's happening is that the mouseover fired by the mouse trailer is
creating a new alert window which reenters the mouse trailer before you've
updated it to indicate the mouse has left the main window?

See also Dainis' window-parenting issue in 300297 ... don't accidentally
traverse the ownership chain :-)
Ok, I found the culprit. When we get the first move event for the dialog,
nsWindow::DispatchMouseEvent heplfully dispatches NS_MOUSE_EXIT for the previous
window at the point of last mouse message in the previous window. This happens
to be on top of the item that fired the dialog in the first place. Then comes
nsEventStateManager that even more helpfully converts the NS_MOUSE_EXIT to
NS_MOUSE_MOVE and causes another dialog to be fired by the onmouseover event.
This is repeated for every move event because the dialog we're on top is a new
one all the time.. Now, I can't see the reason why the exit should be fired for
the previous window. As far as I can see, mouse trailer takes care of that
(unless I've got myself totally confused).
Wow, that one's been there since rev 1.1. Dare I propose it be removed?
Whiteboard: [no l10n impact] → [no l10n impact] Almost there..
When it dispatches the MOUSE_EXIT, it also calls
so you'd need to remove that too, right?

In the situation you describe, I'd expect the fix in bug 297080 to ensure the
MOUSE_EXIT was *not* converted to a MOUSE_MOVE. Is it because gCurrentWindow is
an inner window, not a top-level window?

Maybe the thing to do is make sure the mouse trailer only deals with toplevel
windows, as I described, and then use it here instead of firing the MOUSE_EXIT
from DispatchMouseEvent.
Attachment #189871 - Flags: review?(jst)
Attached patch Win32 patch v2Splinter Review
Ok, this changes mouse trailer to only deal with top level windows and changes
the mouse exit generation in nsWindow to use the actual mouse location instead
of the last known location in the previous window. I've borrowed and adapted
some code by Dainis from bug 300297.
Attachment #189871 - Attachment is obsolete: true
Attachment #190594 - Attachment is obsolete: true
Attachment #192008 - Flags: superreview?(roc)
Comment on attachment 192008 [details] [diff] [review]
Win32 patch v2

this looks like the right thing. Thanks!!!
Attachment #192008 - Flags: superreview?(roc)
Attachment #192008 - Flags: superreview+
Attachment #192008 - Flags: review+
Attachment #192008 - Flags: approval1.8b4?
I've tested the patch, and I think it also fixes bug 297080
Blocks: 297080
Whiteboard: [no l10n impact] Almost there.. → [no l10n impact] Seeking approval
Blocks: 300297
Checked in to trunk, still waiting for approval for branch.
Depends on: 304680
Ere, can you look into this regression?
I have and it doesn't seem related. I've commented in bug 304680. 
Depends on: 304955
This might have caused bug 304955
approval1.8b4 pending clearing up any possible regressions caused by this patch
as seen on the trunk
Patch for bug 304955 waiting for approval. No other known regressions (I don't
believe 304680 is really related).
Attachment #192008 - Flags: approval1.8b4? → approval1.8b4+
Fix checked in to branch too.
Closed: 19 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Whiteboard: [no l10n impact] Seeking approval → [no l10n impact]
Verified fixed with branch build 20050825
(In reply to comment #39)
> Verified fixed with branch build 20050825

Verification for branch is the 'verified1.8' keyword.  Status resolution itself
is for the trunk only.
Resolution: FIXED → ---
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
Verified FIXED using:

on Windows XP SeaMonkey trunk build 2005-09-01-05.
Summary: onmouseover , javascript alert shows twice → onmouseover, javascript alert shows twice
Depends on: 310778
Depends on: 324149
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.