Bug 18677 – Synchronous event dispatch confuses the popup blocker into incorrectly blocking chat popups @ gmail.com

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Geoffrey Garen]

Feng Qian on the GMail team provided this diagnosis.

WORKS (popout escapes blocker):

Click on 'New window' in conversation view
Shift-C keyboard shortcut
Shift-R keyboard shortcut


DOES NOT WORK (grrrrr):

Shift-click on 'compose mail'
Shift-click on thread in threadlist view
Pop out chat

Looks like the issue is nested event handling.

If a event handler triggers another event handler synchronizely, window.event is gone after exiting the nested event handler. The code causing trouble is in handleEvent function of kjs_events.cpp.

windowWrapper->window()->setCurrentEvent(event);
....
windowWrapper->window()->setCurrentEvent(0);

If we save the old event and resotre it after calling the function, it should work. e.g.,

Event* saved_evt = windowWrapper->window()->currentEvent();
windowWrapper->window()->setCurrentEvent(event);
....
windowWrapper->window()->setCurrentEvent(saved_evt);

[Geoffrey Garen]

<rdar://problem/5880560>

[Feng Qian]

Created attachment 20751 [details]
A patch of a layout test

[Feng Qian]

Created attachment 20752 [details]
Patch of kjs_events.cpp

[Geoffrey Garen]

Comment on attachment 20751 [details]
A patch of a layout test

+if (window.layoutTestController) {
+  layoutTestController.dumpAsText();
+}

According to our style guidelines, this one-line if body should not include braces.

[Geoffrey Garen]

Comment on attachment 20752 [details]
Patch of kjs_events.cpp

+        Event* saved_event = windowWrapper->window()->currentEvent();

According to our style guidelines, variable names should use camelCase, so "saved_event" should be named "savedEvent".

[Geoffrey Garen]

Thanks for fixing this!

[Feng Qian]

ggaren, thanks for you review. You can take the patch and make changes as you suggested and land the patch. I don't have SVN checkin right.

[Mark Rowe (bdash)]

I'm making the requested changes and will land this shortly.

[Mark Rowe (bdash)]

Landed in r32525.