Reviewed by Geoff.
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 8 Jul 2006 20:04:09 +0000 (20:04 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 8 Jul 2006 20:04:09 +0000 (20:04 +0000)
        - better fix for http://bugzilla.opendarwin.org/show_bug.cgi?id=9622
          REGRESSION: showModalDialog returnValue ignored, function result is always "undefined"

        * bindings/js/kjs_window.cpp:
        (KJS::showModalDialog): Set the return value after returning from the function if the
        window is not cleared; this is a better way to handle the case where the window does
        not get cleared before returning, and handles some new cases created by slight changes
        in the latest Safari properly too.
        (KJS::Window::clear): Changed logic slightly so we always store the result of getDirect
        into the return value slot -- the old code left the storage untouched if it was 0.
        Also made it only overwrite the return value slot if it's 0.

        * bindings/js/kjs_proxy.h:
        * bindings/js/kjs_proxy.cpp:
        * bindings/js/kjs_window.h:
        * bindings/js/kjs_window.cpp:
        * page/Frame.cpp:
        Roll the previous fix out.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@15239 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/WebCore.xcodeproj/project.pbxproj
WebCore/bindings/js/kjs_proxy.cpp
WebCore/bindings/js/kjs_proxy.h
WebCore/bindings/js/kjs_window.cpp
WebCore/bindings/js/kjs_window.h
WebCore/page/Frame.cpp

index 90c6abc8d30ebb6e019f194f7487c42b81fa1c1c..cf5cbbf6d023ac0f47d9359209758d24336e3e42 100644 (file)
@@ -1,3 +1,26 @@
+2006-07-08  Darin Adler  <darin@apple.com>
+
+        Reviewed by Geoff.
+
+        - better fix for http://bugzilla.opendarwin.org/show_bug.cgi?id=9622
+          REGRESSION: showModalDialog returnValue ignored, function result is always "undefined"
+
+        * bindings/js/kjs_window.cpp:
+        (KJS::showModalDialog): Set the return value after returning from the function if the
+        window is not cleared; this is a better way to handle the case where the window does
+        not get cleared before returning, and handles some new cases created by slight changes
+        in the latest Safari properly too.
+        (KJS::Window::clear): Changed logic slightly so we always store the result of getDirect
+        into the return value slot -- the old code left the storage untouched if it was 0.
+        Also made it only overwrite the return value slot if it's 0.
+
+        * bindings/js/kjs_proxy.h:
+        * bindings/js/kjs_proxy.cpp:
+        * bindings/js/kjs_window.h:
+        * bindings/js/kjs_window.cpp:
+        * page/Frame.cpp:
+        Roll the previous fix out.
+
 2006-07-08  Darin Adler  <darin@apple.com>
 
         - try to fix Windows build
index ec3794a10afc0783217fde2c617aac345a6d0b5d..a8951afe403567c0f2292723ae9bd40a9093dcf0 100644 (file)
                FAE04190097596C9000540BE /* SVGImageLoader.h in Headers */ = {isa = PBXBuildFile; fileRef = FAE0418E097596C9000540BE /* SVGImageLoader.h */; };
 /* End PBXBuildFile section */
 
-/* Begin PBXBuildStyle section */
-               D041FC250A5E04A700841F7F /* Development */ = {
-                       isa = PBXBuildStyle;
-                       buildSettings = {
-                               COPY_PHASE_STRIP = NO;
-                       };
-                       name = Development;
-               };
-               D041FC260A5E04A700841F7F /* Deployment */ = {
-                       isa = PBXBuildStyle;
-                       buildSettings = {
-                               COPY_PHASE_STRIP = YES;
-                       };
-                       name = Deployment;
-               };
-/* End PBXBuildStyle section */
-
 /* Begin PBXContainerItemProxy section */
                DD041FF009D9E3250010AF2A /* PBXContainerItemProxy */ = {
                        isa = PBXContainerItemProxy;
                0867D690FE84028FC02AAC07 /* Project object */ = {
                        isa = PBXProject;
                        buildConfigurationList = 149C284308902B11008A9EFC /* Build configuration list for PBXProject "WebCore" */;
-                       buildSettings = {
-                       };
-                       buildStyles = (
-                               D041FC250A5E04A700841F7F /* Development */,
-                               D041FC260A5E04A700841F7F /* Deployment */,
-                       );
                        hasScannedForEncodings = 1;
                        knownRegions = (
                                English,
index 264cd80a41fc3812f9e3d2f114a8d1d993bd0707..e13a152db0c498cead9fd502e2448a484dc716b5 100644 (file)
@@ -83,15 +83,15 @@ JSValue* KJSProxy::evaluate(const String& filename, int baseLine, const String&
   return 0;
 }
 
-void KJSProxy::clear(bool clearWindowProperties)
-{
-    // Clear resources allocated by the interpreter, and make it ready to be used by another page.
-    // Earlier versions of this function deleted and re-created the window object, but we have to
-    // keep it because JavaScript can keep a reference to the window object and it must be the same.
-    if (m_script) {
-        if (Window* window = Window::retrieveWindow(m_frame))
-            window->clear(clearWindowProperties);
-    }
+void KJSProxy::clear() {
+  // clear resources allocated by the interpreter, and make it ready to be used by another page
+  // We have to keep it, so that the Window object for the frame remains the same.
+  // (we used to delete and re-create it, previously)
+  if (m_script) {
+    Window *win = Window::retrieveWindow(m_frame);
+    if (win)
+        win->clear();
+  }
 }
 
 EventListener* KJSProxy::createHTMLEventHandler(const String& functionName, const String& code, Node* node)
index 8befef489addd35ec77d8eeb927b362a6fb2bede..15f79f1d99e81db0bed4cf6f69614796701532b9 100644 (file)
@@ -41,7 +41,7 @@ public:
     KJSProxy(Frame*);
     ~KJSProxy();
     KJS::JSValue* evaluate(const String& filename, int baseLine, const String& code, Node*);
-    void clear(bool clearWindowProperties);
+    void clear();
     EventListener* createHTMLEventHandler(const String& functionName, const String& code, Node*);
 #if SVG_SUPPORT
     EventListener* createSVGEventHandler(const String& functionName, const String& code, Node*);
index b2335ea3c81222c16ef16c90bda6794d1c9651f8..51b9a0bb5354ef4346d780cb104adea4882d400a 100644 (file)
@@ -598,7 +598,7 @@ static bool canShowModalDialogNow(const Window *window)
     return frame && static_cast<BrowserExtension *>(frame->browserExtension())->canRunModalNow();
 }
 
-static JSValue *showModalDialog(ExecState *exec, Window *openerWindow, const List &args)
+static JSValue* showModalDialog(ExecState* exec, Window* openerWindow, const List& args)
 {
     UString URL = args[0]->toString(exec);
 
@@ -651,16 +651,26 @@ static JSValue *showModalDialog(ExecState *exec, Window *openerWindow, const Lis
     wargs.locationBarVisible = false;
     wargs.fullscreen = false;
     
-    Frame *dialogPart = createNewWindow(exec, openerWindow, URL, "", wargs, args[1]);
-    if (!dialogPart)
+    Frame* dialogFrame = createNewWindow(exec, openerWindow, URL, "", wargs, args[1]);
+    if (!dialogFrame)
         return jsUndefined();
 
-    Window *dialogWindow = Window::retrieveWindow(dialogPart);
-    JSValue *returnValue = jsUndefined();
+    Window* dialogWindow = Window::retrieveWindow(dialogFrame);
+
+    // 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);
-    static_cast<BrowserExtension *>(dialogPart->browserExtension())->runModal();
-    dialogWindow->setReturnValueSlot(NULL);
-    return returnValue;
+    static_cast<BrowserExtension *>(dialogFrame->browserExtension())->runModal();
+    dialogWindow->setReturnValueSlot(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,
+    // and in that case, there's no harm in trying again (no benefit either).
+    if (!returnValue)
+        returnValue = dialogWindow->getDirect("returnValue");
+
+    return returnValue ? returnValue : jsUndefined();
 }
 
 JSValue *Window::getValueProperty(ExecState *exec, int token) const
@@ -1323,25 +1333,22 @@ JSUnprotectedEventListener *Window::getJSUnprotectedEventListener(JSValue *val,
   return new JSUnprotectedEventListener(object, this, html);
 }
 
-void Window::clear(bool clearWindowProperties)
+void Window::clear()
 {
-    JSLock lock;
+  JSLock lock;
 
-    if (m_returnValueSlot)
-        if (JSValue* returnValue = getDirect("returnValue"))
-            *m_returnValueSlot = returnValue;
+  if (m_returnValueSlot && !*m_returnValueSlot)
+    *m_returnValueSlot = getDirect("returnValue");
 
-    if (clearWindowProperties) {
-        clearAllTimeouts();
-        clearProperties();
-        setPrototype(JSDOMWindowProto::self()); // clear the prototype
+  clearAllTimeouts();
+  clearProperties();
+  setPrototype(JSDOMWindowProto::self()); // clear the prototype
 
-        // there's likely to be lots of garbage now
-        Collector::collect();
+  // there's likely to be lots of garbage now
+  Collector::collect();
 
-        // Now recreate a working global object for the next URL that will use us
-        interpreter()->initGlobalObject();
-    }
+  // Now recreate a working global object for the next URL that will use us
+  interpreter()->initGlobalObject();
 }
 
 void Window::setCurrentEvent(Event *evt)
index 270dad0cf9fbe86f67ea57386465c2cc8b3637c1..3619ed805a9f96e255254a617cd001c7eee208b3 100644 (file)
@@ -138,7 +138,7 @@ namespace KJS {
     BarInfo *toolbar(ExecState*) const;
     JSEventListener *getJSEventListener(JSValue*, bool html = false);
     JSUnprotectedEventListener *getJSUnprotectedEventListener(JSValue*, bool html = false);
-    void clear(bool clearWindowProperties);
+    void clear();
     virtual UString toString(ExecState *) const;
 
     // Set the current "event" object
index c1b66a7b30473617278a84e51104872f1e768ed5..2c116501469b8a480293a09d7f5cce482d2a1186 100644 (file)
@@ -437,8 +437,8 @@ void Frame::clear(bool clearWindowProperties)
   }
 
   // Moving past doc so that onUnload works.
-  if (d->m_jscript)
-    d->m_jscript->clear(clearWindowProperties);
+  if (clearWindowProperties && d->m_jscript)
+    d->m_jscript->clear();
 
   if (d->m_view)
     d->m_view->clear();