Commit a1e24df2 authored by thatcher's avatar thatcher

WebCore:

        Reviewed by Sam.

        <rdar://problem/5472970> REGRESSION (r24276): TinyMCE popups show an empty window with no content

        Accessing the document of a window before the load finished would cause the window
        object to hold onto the initial empty document, and never switch over to the real document
        once the load finished. This regression was caused by r24276 which added a check to prevent
        clearing the window object when the load finished. The absence of this clear allowed the
        dialogArguments set with showModalDialog to persist on the window after the load. However,
        not clearing the window would keep other properties (and the empty document object) around.

        So the fix is to store away the dialog arguments that were passed to showModalDialog and
        put them back on the window object in the dialogArguments property each time
        the window is cleared.

        * bindings/js/kjs_window.cpp:
        (KJS::createWindow): No longer put dialogArguments on the window here.
        (KJS::showModalDialog): Put dialogArguments on the window and call
        setDialogArgumentsAndReturnValueSlot to remember the arguments.
        (KJS::Window::clear): Put m_dialogArguments back on the window as dialogArguments.
        (KJS::WindowFunc::callAsFunction): Call the new setDialogArgumentsAndReturnValue.
        (KJS::Window::setDialogArgumentsAndReturnValue): Store the arguments in m_dialogArguments.
        * bindings/js/kjs_window.h: Rename setReturnValueSlot to setDialogArgumentsAndReturnValueSlot.
        * manual-tests/modal-dialog-arguments.html: Confirmed that this test still passes.

        Reverted r24276 which was all the changes in FrameLoader.cpp and FrameLoader.h.

        * loader/FrameLoader.cpp:
        (WebCore::FrameLoader::FrameLoader): Remove m_shouldClearWindowProperties.
        (WebCore::FrameLoader::createWindow): Remove the call to setShouldClearWindowProperties.
        (WebCore::FrameLoader::clear): No longer check m_shouldClearWindowProperties, clear the
        window whenever clearWindowProperties is set.
        (WebCore::FrameLoader::begin): Remove m_shouldClearWindowProperties.
        (WebCore::FrameLoader::open): Ditto.
        * loader/FrameLoader.h: Remove m_shouldClearWindowProperties.

LayoutTests:

        Reviewed by Sam.

        <rdar://problem/5472970> REGRESSION (r24276): TinyMCE popups show an empty window with no content

        * fast/dom/Document/early-document-access.html: Added.
        * fast/dom/Document/resources: Added.
        * fast/dom/Document/resources/early-document-access-popup.html: Added.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@25576 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent a919aacb
2007-09-14 Timothy Hatcher <timothy@apple.com>
Reviewed by Sam.
<rdar://problem/5472970> REGRESSION (r24276): TinyMCE popups show an empty window with no content
* fast/dom/Document/early-document-access.html: Added.
* fast/dom/Document/resources: Added.
* fast/dom/Document/resources/early-document-access-popup.html: Added.
2007-09-13 Geoffrey Garen <ggaren@apple.com>
Reviewed by Adam Roben.
<html>
<head>
<script>
function test() {
if (window.layoutTestController) {
layoutTestController.setCanOpenWindows();
layoutTestController.dumpAsText();
layoutTestController.waitUntilDone();
}
newWindow = window.open('resources/early-document-access-popup.html');
if (newWindow) {
newWindow.document; // access the document
setTimeout(fail, 5000); // fail if we take longer than 5 seconds
} else {
document.body.innerText = 'FAIL: window.open failed to make a window.';
layoutTestController.notifyDone();
}
}
function done() {
newWindow.close();
if (window.layoutTestController)
setTimeout("layoutTestController.notifyDone()", 100);
}
function pass() {
document.body.innerText = 'PASS';
done();
}
function fail() {
document.body.innerText = 'FAIL';
done();
}
</script>
</head>
<body onload="test()">
FAIL: TEST DID NOT RUN!
</body>
</html>
<html>
<head>
<script>
function test() {
if (document.body.getAttribute('test') == 'pass')
window.opener.pass();
else
window.opener.fail();
}
</script>
</head>
<body onload="test()" test="pass"></body>
</html>
2007-09-14 Timothy Hatcher <timothy@apple.com>
Reviewed by Sam.
<rdar://problem/5472970> REGRESSION (r24276): TinyMCE popups show an empty window with no content
Accessing the document of a window before the load finished would cause the window
object to hold onto the initial empty document, and never switch over to the real document
once the load finished. This regression was caused by r24276 which added a check to prevent
clearing the window object when the load finished. The absence of this clear allowed the
dialogArguments set with showModalDialog to persist on the window after the load. However,
not clearing the window would keep other properties (and the empty document object) around.
So the fix is to store away the dialog arguments that were passed to showModalDialog and
put them back on the window object in the dialogArguments property each time
the window is cleared.
* bindings/js/kjs_window.cpp:
(KJS::createWindow): No longer put dialogArguments on the window here.
(KJS::showModalDialog): Put dialogArguments on the window and call
setDialogArgumentsAndReturnValueSlot to remember the arguments.
(KJS::Window::clear): Put m_dialogArguments back on the window as dialogArguments.
(KJS::WindowFunc::callAsFunction): Call the new setDialogArgumentsAndReturnValue.
(KJS::Window::setDialogArgumentsAndReturnValue): Store the arguments in m_dialogArguments.
* bindings/js/kjs_window.h: Rename setReturnValueSlot to setDialogArgumentsAndReturnValueSlot.
* manual-tests/modal-dialog-arguments.html: Confirmed that this test still passes.
Reverted r24276 which was all the changes in FrameLoader.cpp and FrameLoader.h.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::FrameLoader): Remove m_shouldClearWindowProperties.
(WebCore::FrameLoader::createWindow): Remove the call to setShouldClearWindowProperties.
(WebCore::FrameLoader::clear): No longer check m_shouldClearWindowProperties, clear the
window whenever clearWindowProperties is set.
(WebCore::FrameLoader::begin): Remove m_shouldClearWindowProperties.
(WebCore::FrameLoader::open): Ditto.
* loader/FrameLoader.h: Remove m_shouldClearWindowProperties.
2007-09-14 Brady Eidson <beidson@apple.com>
How about a build fix that works on *all* platforms?
......@@ -94,7 +94,8 @@ struct WindowPrivate {
Window::UnprotectedListenersMap jsUnprotectedHTMLEventListeners;
mutable Location* loc;
WebCore::Event *m_evt;
JSValue **m_returnValueSlot;
JSValue* m_dialogArguments;
JSValue** m_returnValueSlot;
typedef HashMap<int, DOMWindowTimer*> TimeoutsMap;
TimeoutsMap m_timeouts;
};
......@@ -358,7 +359,7 @@ static float floatFeature(const HashMap<String, String> &features, const char *k
}
static Frame* createWindow(ExecState* exec, Frame* openerFrame, const String& url,
const String& frameName, const WindowFeatures& windowFeatures, JSValue* dialogArgs)
const String& frameName, const WindowFeatures& windowFeatures)
{
Frame* activeFrame = Window::retrieveActive(exec)->impl()->frame();
......@@ -370,7 +371,7 @@ static Frame* createWindow(ExecState* exec, Frame* openerFrame, const String& ur
// FIXME: It's much better for client API if a new window starts with a URL, here where we
// know what URL we are going to open. Unfortunately, this code passes the empty string
// for the URL, but there's a reason for that. Before loading we have to set up the opener,
// openedByJS, and dialogArguments values. Also, to decide whether to use the URL we currently
// openedByDOM, and dialogArguments values. Also, to decide whether to use the URL we currently
// do an isSafeScript call using the window we create, which can't be done before creating it.
// We'd have to resolve all those issues to pass the URL instead of "".
......@@ -384,9 +385,6 @@ static Frame* createWindow(ExecState* exec, Frame* openerFrame, const String& ur
Window* newWindow = Window::retrieveWindow(newFrame);
if (dialogArgs)
newWindow->putDirect("dialogArguments", dialogArgs);
if (!url.startsWith("javascript:", false) || newWindow->isSafeScript(exec)) {
String completedURL = url.isEmpty() ? url : activeFrame->document()->completeURL(url);
bool userGesture = static_cast<ScriptInterpreter *>(exec->dynamicInterpreter())->wasRunByUserGesture();
......@@ -476,18 +474,20 @@ static JSValue* showModalDialog(ExecState* exec, Window* openerWindow, const Lis
wargs.locationBarVisible = false;
wargs.fullscreen = false;
Frame* dialogFrame = createWindow(exec, frame, valueToStringWithUndefinedOrNullCheck(exec, args[0]), "", wargs, args[1]);
Frame* dialogFrame = createWindow(exec, frame, valueToStringWithUndefinedOrNullCheck(exec, args[0]), "", wargs);
if (!dialogFrame)
return jsUndefined();
Window* dialogWindow = Window::retrieveWindow(dialogFrame);
dialogWindow->putDirect("dialogArguments", args[1]);
// Get the return value either just before clearing the dialog window's
// properties (in Window::clear), or when on return from runModal.
JSValue* returnValue = 0;
dialogWindow->setReturnValueSlot(&returnValue);
dialogWindow->setDialogArgumentsAndReturnValueSlot(args[1], &returnValue);
dialogFrame->page()->chrome()->runModal();
dialogWindow->setReturnValueSlot(0);
dialogWindow->setDialogArgumentsAndReturnValueSlot(0, 0);
// If we don't have a return value, get it now.
// Either Window::clear was not called yet, or there was no return value,
......@@ -1048,6 +1048,9 @@ void Window::clear()
if (Frame* frame = impl()->frame())
frame->scriptProxy()->interpreter()->initGlobalObject();
if (d->m_dialogArguments)
putDirect("dialogArguments", d->m_dialogArguments);
// there's likely to be lots of garbage now
gcController().garbageCollectSoon();
}
......@@ -1300,7 +1303,7 @@ JSValue *WindowFunc::callAsFunction(ExecState *exec, JSObject *thisObj, const Li
windowFeatures.height = windowRect.height();
windowFeatures.width = windowRect.width();
frame = createWindow(exec, frame, urlString, frameName, windowFeatures, 0);
frame = createWindow(exec, frame, urlString, frameName, windowFeatures);
if (!frame)
return jsUndefined();
......@@ -1443,9 +1446,10 @@ void Window::updateLayout() const
docimpl->updateLayoutIgnorePendingStylesheets();
}
void Window::setReturnValueSlot(JSValue **slot)
{
d->m_returnValueSlot = slot;
void Window::setDialogArgumentsAndReturnValueSlot(JSValue* dialogArgs, JSValue** returnValueSlot)
{
d->m_dialogArguments = dialogArgs;
d->m_returnValueSlot = returnValueSlot;
}
////////////////////// ScheduledAction ////////////////////////
......
......@@ -128,7 +128,7 @@ namespace KJS {
void setCurrentEvent(WebCore::Event*);
// Set a place to put a dialog return value when the window is cleared.
void setReturnValueSlot(JSValue **slot);
void setDialogArgumentsAndReturnValueSlot(JSValue* arguments, JSValue** returnValueSlot);
typedef HashMap<JSObject*, WebCore::JSEventListener*> ListenersMap;
typedef HashMap<JSObject*, WebCore::JSUnprotectedEventListener*> UnprotectedListenersMap;
......
......@@ -227,7 +227,6 @@ FrameLoader::FrameLoader(Frame* frame, FrameLoaderClient* client)
, m_isLoadingMainResource(false)
, m_cancellingWithLoadInProgress(false)
, m_needsClear(false)
, m_shouldClearWindowProperties(true)
, m_receivedData(false)
, m_encodingWasChosenByUser(false)
, m_containsPlugIns(false)
......@@ -317,8 +316,6 @@ Frame* FrameLoader::createWindow(const FrameLoadRequest& request, const WindowFe
if (request.frameName() != "_blank")
frame->tree()->setName(request.frameName());
frame->loader()->setShouldClearWindowProperties(false);
page->chrome()->setToolbarsVisible(features.toolBarVisible || features.locationBarVisible);
page->chrome()->setStatusbarVisible(features.statusBarVisible);
page->chrome()->setScrollbarsVisible(features.scrollbarsVisible);
......@@ -788,7 +785,7 @@ void FrameLoader::clear(bool clearWindowProperties)
}
// Do this after detaching the document so that the unload event works.
if (clearWindowProperties && m_shouldClearWindowProperties) {
if (clearWindowProperties) {
m_frame->clearScriptProxy();
m_frame->clearDOMWindow();
}
......@@ -870,7 +867,6 @@ void FrameLoader::begin(const KURL& url, bool dispatch)
dispatchWindowObjectAvailable();
m_needsClear = true;
m_shouldClearWindowProperties = true;
m_isComplete = false;
m_didCallImplicitClose = false;
m_isLoadingMainResource = true;
......@@ -2685,7 +2681,6 @@ void FrameLoader::open(CachedPage& cachedPage)
document->setInPageCache(false);
m_needsClear = true;
m_shouldClearWindowProperties = true;
m_isComplete = false;
m_didCallImplicitClose = false;
m_outgoingReferrer = URL.url();
......
......@@ -562,8 +562,6 @@ namespace WebCore {
void dispatchDidFinishLoading(DocumentLoader*, unsigned long identifier);
bool dispatchDidLoadResourceFromMemoryCache(DocumentLoader*, const ResourceRequest&, const ResourceResponse&, int length);
void setShouldClearWindowProperties(bool shouldClearWindowProperties) { m_shouldClearWindowProperties = shouldClearWindowProperties; }
Frame* m_frame;
FrameLoaderClient* m_client;
......@@ -616,7 +614,6 @@ namespace WebCore {
OwnPtr<ScheduledRedirection> m_scheduledRedirection;
bool m_needsClear;
bool m_shouldClearWindowProperties;
bool m_receivedData;
bool m_encodingWasChosenByUser;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment