Closed Bug 1407700 Opened 3 years ago Closed 3 years ago

onmouseover alert() crashes browser tab


(Core :: DOM: Events, defect, P1)

58 Branch



Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 --- unaffected
firefox58 --- fixed


(Reporter: george, Assigned: stone)



(Keywords: crash, crashreportid)


(3 files, 1 obsolete file)

Attached file crash.html
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20171010220102

Steps to reproduce:

1. Open the attached HTML file.
2. Hover your mouse over the text.
3. Click 'OK' to close the alert dialog that appears.

Actual results:

The browser tab crashes.

Crash reports:
  * Ubuntu 17.04:
  * Windows 10:

Expected results:

The tab shouldn't have crashed.
Component: Untriaged → DOM: Events
Keywords: crash, crashreportid
Product: Firefox → Core
Flags: needinfo?(sshih)
Something to do with MaybeDispatchCoalescedMouseMoveEvents -> stone
Assignee: nobody → sshih
Flags: needinfo?(sshih)
Comment on attachment 8917688 [details] [diff] [review]
Part2: Handle the case to re-entry event loop for mousemove coalescing

Dropping events may be a bad idea. Trying to revise it.
Attachment #8917688 - Flags: review?(bugs)
Attachment #8917687 - Flags: review?(bugs)
(In reply to George Pîrlea from comment #0)
> Created attachment 8917458 [details]
> crash.html
Thanks for identifying the steps and creating the test case.
Attachment #8917687 - Flags: review?(bugs)
Attachment #8918067 - Flags: review?(bugs)
We might have a similar problem of wheel event coalescing that dispatches disorder events while reentrying the event loop. Created bug 1408232 to fix it.
See Also: → 1408232
(In reply to Ming-Chou Shih [:stone] from comment #7)
> Created attachment 8918067 [details] [diff] [review]
> Part2: Handle the case to re-entry event loop for mousemove coalescing

This is caused by reentrying the event loop and touch the same hash table while traversing it and dispatching an event. When dispatching the coalesced events, move them to another queue first and then start dispatching events.

When receiving a new mousemove which we can't coalesce, we should dispatch the coalesced event with the same pointer id. When receiving a new mouse event other than mouse move, we should dispatch all coalesced events. We could put the new mouse event (other than mousemove) in the same queue to make sure we won't dispatch it with incorrect order while reentrying the event loop.
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0

I have tested this issue on Windows 10 x64, using the STR provided in comment 0, performed a regression, the result as follows:

Last good revision: 212ceaf885b0d8750201f816030ad0db8391c7d7
First bad revision: 5e0a1d82ef670c537b7b5c60ba0f2606a9f04ef7

Looks like the following bug has the changes which introduced the regression:
Comment on attachment 8917687 [details] [diff] [review]
Part1: Revise CoalescedInputData interfaces.

>+  UniquePtr<InputEventType> GetCoalescedEvent()
>   {
>-    return mCoalescedInputEvent.get();
>+    return Move(mCoalescedInputEvent);
>   }
The method should be called something like
TakeCoalescedEvent to hint that it actually clears the member variable on the object.
Attachment #8917687 - Flags: review?(bugs) → review+
Sorry about delay with review.
Attachment #8918067 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] (work week Oct 16-20) from comment #12)
> Sorry about delay with review.

It's okay. I know you are in the work week and thanks for the review.
Pushed by
Part1: Revise CoalescedInputData interfaces. r=smaug.
Part2: Handle the case to re-entry event loop for mousemove coalescing. r=smaug.
You need to log in before you can comment on or make changes to this bug.