Created attachment 80230 [details] Sample web pages/scripts to reproduce showModalDialog does not correctly return the defined returnValue in case domain relaxing is used, even if the opener window and the modal dialog content are properly set to the same domain. Ver: 534.17+ In some web applications content from different systems are embedded inside of iframes and need to communicate with each other, so it is common to have domain relaxing enabled, to allow JavaScript communication across the iframes. We discovered that showModalDialog does not correctly return the defined returnValue in case domain relaxing is used, even if the opener window and the modal dialog content are properly set to the same domain. Prerequisites to reproduce the issue: 1. Copy the modaldialog-folder containing the three files index.html, dialogbroken.html and dialogworkaround.html to a local webserver 2. Add the following line to your /etc/hosts 127.0.0.1 localhost.test.domain 3.Disable the proxy server in the browser (if any) Steps to reproduce: 1. Open WebKit and load the page from your server using http://localhost/modaldialog/ 2. Click on "Open modal dialog" 3. Click on "Close and return TEST" 4. The alert-window shows the proper return value "TEST" 5. Now change the URL to http://localhost.test.domain/modaldialog/ 6. Click on "Open modal dialog" again 7. Click on "Close and return TEST" The alert-window now returns "undefined" instead of "TEST" As a workaround it is possible to store the return value in the parent window, which can be accessed using window.opener, and to adapt the code which reads the return value accordingly.
<rdar://problem/8629478>
Moreover, dialogArguments variable isn't available in modal dialog in this case.
> Moreover, dialogArguments variable isn't available in modal dialog in this case. OK, that doesn't work in Firefox or IE either. Makes sense, since the engine doesn't yet know if the dialog is going to use domain relaxing, and passing data cross origin is not allowed.
Created attachment 104766 [details] proposed fix
Created attachment 104769 [details] proposed fix Changed to actually successfully print an error message.
Comment on attachment 104769 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=104769&action=review r=me with those changes > Source/WebCore/ChangeLog:19 > + dismissed. A dialog can navigate itself, and it also creates a new JSDOMWindow on firt load s/firt/first/ > Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:678 > + JSDOMWindow* globalObject = toJSDOMWindow(m_frame.get(), normalWorld(m_exec->globalData())); > + if (!globalObject) > + return jsUndefined(); > + if (!asJSDOMWindow(m_exec->lexicalGlobalObject())->allowsAccessFrom(globalObject->globalExec())) > return jsUndefined(); > Identifier identifier(m_exec, "returnValue"); > PropertySlot slot; > - if (!m_globalObject->JSGlobalObject::getOwnPropertySlot(m_exec, identifier, slot)) > + if (!globalObject->JSGlobalObject::getOwnPropertySlot(m_exec, identifier, slot)) There's no need to do this allowsAccessFrom check ourselves; it's built into the window object. Just call the normal getOwnPropertySlot, instead of skipping window checks by calling JSGlobalObject::getOwnPropertySlot directly. By the way, make sure to call out this additional security check in your ChangeLog, and explain why you added it (to match Firefox).
Committed <http://trac.webkit.org/changeset/93567>
This broke a GTK+ API test. Our DOM objects from the GObject bindings are not being collected anymore, since we don't get a frame destruction notification when doing: create view, get refs to DOM objects, destroy view. I guess this is because of the keepAlive() call added to pageDestroyed... not sure if the bug is ours or not, but in any case this has changed behavior without adding new tests.
The keepAlive call was always there, just called indirectly. It's quite possible that there is a real bug introduced here, and one that affects platforms other than Gtk, but since the only known change is on Gtk, it's not practical for me to investigate what's going on.
(In reply to comment #9) > The keepAlive call was always there, just called indirectly. keepAlive() is only called for the JS bindings. Our unit tests do not have any JS, so that's likely the reason they broke (since they never had the keepAlive() call in the first place). > > It's quite possible that there is a real bug introduced here, and one that affects platforms other than Gtk, but since the only known change is on Gtk, it's not practical for me to investigate what's going on. I guess this might or might not affect other !JS bindings, but not sure.