40036 2010-06-01 18:19:25 -0700 Make WebChromeClient::closeWindowSoon work for all RunLoop modes 2010-09-14 19:26:29 -0700 1 1 1 Unclassified WebKit WebKit Misc. 528+ (Nightly build) PC OS X 10.5 RESOLVED INVALID P2 Normal --- 35350 1 prasadt webkit-unassigned ap darin dimich levin oldest_to_newest 233016 0 prasadt 2010-06-01 18:19:25 -0700 performSelector:withObject:afterDelay method is being used to post the _closeWindow message to the queue. Those message will only get dispatched if the RunLoop is in NSDefaultRunLoopMode. Since RunLoop is in NSModalPanelRunLoopMode mode when a modal window is being displayed, you can't close the window. Replaced the call with the inModes version to allow the close window message to be dispatched in all RunLoop modes. 233019 1 57611 prasadt 2010-06-01 18:25:49 -0700 Created attachment 57611 Patch to make closeWindowSoon work in all RunLoop modes. 233301 2 levin 2010-06-02 09:37:00 -0700 This is follow up from http://www.mail-archive.com/webkit-dev@lists.webkit.org/msg11394.html as understand it. 234541 3 57611 darin 2010-06-04 22:52:35 -0700 Comment on attachment 57611 Patch to make closeWindowSoon work in all RunLoop modes. I don’t think this change is correct or helpful. For example, I don’t think we want to close a window and run all the code that triggers inside an NSConnectionReply modal event loop. I don’t understand the benefit side of this patch either. 234886 4 prasadt 2010-06-07 09:36:00 -0700 (In reply to comment #3) > (From update of attachment 57611 [details]) > I don’t think this change is correct or helpful. For example, I don’t think we want to close a window and run all the code that triggers inside an NSConnectionReply modal event loop. > > I don’t understand the benefit side of this patch either. When you write a WebKit application that implements showModalDialog, you don't have a way to close the window from script. The close call ends up in the closeWindowSoon() function and the _closeWindow() doesn't get dispatched because the run loop is in NSModalPanelRunLoopMode mode. I didn't realize that dispatching these calls is inappropriate in certain modes. I could make this fix more targeted and make it to be: [m_webView performSelector:@selector(_closeWindow) withObject:nil afterDelay:0.0 inModes:[NSArray arrayWithObjects:NSDefaultRunLoopMode, NSModalPanelRunLoopMode]]; With that change the only new mode we'll be closing windows would be in NSModalPanelRunLoopMode. Does that seem like a correct change? I ran into this issue when adding showModalDialog to DumpRenderTree as part of the bug fix for - https://bugs.webkit.org/show_bug.cgi?id=35350. You'll notice I have a patch attached to that bug that includes this change. I decided to factor out this particular change into a separate patch as this is the more sensitive part of that big patch. Thanks, Prasad 253442 5 57611 dimich 2010-07-19 16:06:34 -0700 Comment on attachment 57611 Patch to make closeWindowSoon work in all RunLoop modes. Removing r? since the root bug 35350 has landed and it seems a different approach was selected to support showModalDialog testing. Please update if this is not so. 279332 6 prasadt 2010-09-14 19:26:29 -0700 We don't need this change anymore. 57611 2010-06-01 18:25:49 -0700 2010-07-19 16:06:34 -0700 Patch to make closeWindowSoon work in all RunLoop modes. closeWindowSoon.txt text/plain 1816 prasadt SW5kZXg6IFdlYktpdC9tYWMvV2ViQ29yZVN1cHBvcnQvV2ViQ2hyb21lQ2xpZW50Lm1tCj09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0KLS0tIFdlYktpdC9tYWMvV2ViQ29yZVN1cHBvcnQvV2ViQ2hyb21lQ2xpZW50Lm1tCShy ZXZpc2lvbiA2MDUxNCkKKysrIFdlYktpdC9tYWMvV2ViQ29yZVN1cHBvcnQvV2ViQ2hyb21lQ2xp ZW50Lm1tCSh3b3JraW5nIGNvcHkpCkBAIC0zNTcsNyArMzU3LDkgQEAKIAogICAgIFttX3dlYlZp ZXcgc2V0R3JvdXBOYW1lOm5pbF07CiAgICAgW21fd2ViVmlldyBzdG9wTG9hZGluZzpuaWxdOwot ICAgIFttX3dlYlZpZXcgcGVyZm9ybVNlbGVjdG9yOkBzZWxlY3RvcihfY2xvc2VXaW5kb3cpIHdp dGhPYmplY3Q6bmlsIGFmdGVyRGVsYXk6MC4wXTsKKyAgICBbbV93ZWJWaWV3IHBlcmZvcm1TZWxl Y3RvcjpAc2VsZWN0b3IoX2Nsb3NlV2luZG93KSB3aXRoT2JqZWN0Om5pbCBhZnRlckRlbGF5OjAu MAorICAgICAgICBpbk1vZGVzOltOU0FycmF5IGFycmF5V2l0aE9iamVjdHM6TlNEZWZhdWx0UnVu TG9vcE1vZGUsIE5TTW9kYWxQYW5lbFJ1bkxvb3BNb2RlLAorICAgICAgICAgICAgTlNDb25uZWN0 aW9uUmVwbHlNb2RlLCBOU0V2ZW50VHJhY2tpbmdSdW5Mb29wTW9kZSwgbmlsXV07CiB9CiAKIHZv aWQgV2ViQ2hyb21lQ2xpZW50OjpydW5KYXZhU2NyaXB0QWxlcnQoRnJhbWUqIGZyYW1lLCBjb25z dCBTdHJpbmcmIG1lc3NhZ2UpCkluZGV4OiBXZWJLaXQvbWFjL0NoYW5nZUxvZwo9PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 Ci0tLSBXZWJLaXQvbWFjL0NoYW5nZUxvZwkocmV2aXNpb24gNjA1MjEpCisrKyBXZWJLaXQvbWFj L0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBACisyMDEwLTA2LTAxICBQ cmFzYWQgVGFtbWFuYSAgPHByYXNhZHRAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIE1ha2UgV2ViQ2hyb21lQ2xpZW50OjpjbG9z ZVdpbmRvd1Nvb24gd29yayBmb3IgYWxsIFJ1bkxvb3AgbW9kZXMKKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTQwMDM2CisKKyAgICAgICAgKiBXZWJDb3Jl U3VwcG9ydC9XZWJDaHJvbWVDbGllbnQubW06CisgICAgICAgIChXZWJDaHJvbWVDbGllbnQ6OmNs b3NlV2luZG93U29vbik6IHBlcmZvcm1TZWxlY3Rvcjp3aXRoT2JqZWN0OmFmdGVyRGVsYXkgbWV0 aG9kIGlzIGJlaW5nIHVzZWQgdG8KKyAgICAgICAgcG9zdCB0aGUgX2Nsb3NlV2luZG93IG1lc3Nh Z2UgdG8gdGhlIHF1ZXVlLiAgVGhvc2UgbWVzc2FnZSB3aWxsIG9ubHkgZ2V0IGRpc3BhdGNoZWQg aWYgdGhlCisgICAgICAgIFJ1bkxvb3AgaXMgaW4gTlNEZWZhdWx0UnVuTG9vcE1vZGUuICBTaW5j ZSBSdW5Mb29wIGlzIGluIE5TTW9kYWxQYW5lbFJ1bkxvb3BNb2RlIG1vZGUgd2hlbiBhCisgICAg ICAgIG1vZGFsIHdpbmRvdyBpcyBiZWluZyBkaXNwbGF5ZWQsIHlvdSBjYW4ndCBjbG9zZSB0aGUg d2luZG93LiAgUmVwbGFjZWQgdGhlIGNhbGwgd2l0aCB0aGUgaW5Nb2RlcworICAgICAgICB2ZXJz aW9uIHRvIGFsbG93IHRoZSBjbG9zZSB3aW5kb3cgbWVzc2FnZSB0byBiZSBkaXNwYXRjaGVkIGlu IGFsbCBSdW5Mb29wIG1vZGVzLgorCiAyMDEwLTA2LTAxICBEYXZpZCBIeWF0dCAgPGh5YXR0QGFw cGxlLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBBbmRlcnMgQ2FybHNzb24uCg==