Bug 18677

Summary: Synchronous event dispatch confuses the popup blocker into incorrectly blocking chat popups @ gmail.com
Product: WebKit Reporter: Geoffrey Garen <ggaren>
Component: WebCore Misc.Assignee: Mark Rowe (bdash) <mrowe>
Severity: Normal CC: ggaren, mrowe
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Description Flags
A patch of a layout test
ggaren: review+
Patch of kjs_events.cpp ggaren: review+

Description Geoffrey Garen 2008-04-22 11:31:34 PDT
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.


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

Event* saved_evt = windowWrapper->window()->currentEvent();
Comment 1 Geoffrey Garen 2008-04-22 11:32:17 PDT
Comment 2 Feng Qian 2008-04-22 11:59:48 PDT
Created attachment 20751 [details]
A patch of a layout test
Comment 3 Feng Qian 2008-04-22 12:00:49 PDT
Created attachment 20752 [details]
Patch of kjs_events.cpp
Comment 4 Geoffrey Garen 2008-04-23 07:51:50 PDT
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.
Comment 5 Geoffrey Garen 2008-04-23 07:52:38 PDT
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".
Comment 6 Geoffrey Garen 2008-04-23 07:52:58 PDT
Thanks for fixing this!
Comment 7 Feng Qian 2008-04-23 09:05:46 PDT
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.
Comment 8 Mark Rowe (bdash) 2008-04-24 17:13:38 PDT
I'm making the requested changes and will land this shortly.
Comment 9 Mark Rowe (bdash) 2008-04-24 17:50:13 PDT
Landed in r32525.