Summary: | showModalDialog does not correctly return the defined returnValue in case domain relaxing is used | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brent <brentm00> | ||||||||
Component: | WebCore JavaScript | Assignee: | Alexey Proskuryakov <ap> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, ap, xan.lopez | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Mac | ||||||||||
OS: | OS X 10.6 | ||||||||||
Bug Depends on: | 66749, 66894, 66895 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Brent
2011-01-26 13:50:52 PST
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. |