WebCore:
authortristan <tristan@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Oct 2007 19:22:11 +0000 (19:22 +0000)
committertristan <tristan@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Oct 2007 19:22:11 +0000 (19:22 +0000)
        Reviewed by Darin Adler.

        <rdar://problem/5555053> REGRESSION:9A581: Window disappears when opening http://research.microsoft.com/users/darkok/
        The problem was caused by checkin r24654. This change moved explicit bounds checking into adjustWindowRect
        but failed to account for bounds checking (instead replaced with bounds clipping).  This caused issues
        when NaN was used. This patch goes one step further and does NaN checking to prevent the possibility of
        setting window bounds to NaN before an update occurs.

        Test: fast/dom/Window/window-resize-nan.html

        * bindings/js/kjs_window.cpp:
        (KJS::adjustWindowRect):
        Added a new parameter, pendingChanges, which takes pending changes to the window
        rect, and if they are valid (not NaN) sets them on window.

        (KJS::WindowFunc::callAsFunction):
        Adjusted uses of adjustWindowRect to take new update parameter.

LayoutTests:

        Reviewed by Darin Adler.

        Added new test cases to handle non-number input to window adjusting
        functions like resizeTo, resizeBy, moveTo, and moveBy for
        <rdar://problem/5555053>.

        * fast/dom/Window/window-resize-nan-expected.txt: Added.
        * fast/dom/Window/window-resize-nan.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/Window/window-resize-nan-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Window/window-resize-nan.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/bindings/js/kjs_window.cpp

index 1a620c4618afc4226ead9e533523ca745b4dcb10..78ea2d7237e42aab01c48ece70b74a0cb985f830 100644 (file)
@@ -1,3 +1,14 @@
+2007-10-26  Tristan O'Tierney  <tristan@apple.com>
+
+        Reviewed by Darin Adler.
+        
+        Added new test cases to handle non-number input to window adjusting
+        functions like resizeTo, resizeBy, moveTo, and moveBy for 
+        <rdar://problem/5555053>.
+
+        * fast/dom/Window/window-resize-nan-expected.txt: Added.
+        * fast/dom/Window/window-resize-nan.html: Added.
+
 2007-10-26  Darin Adler  <darin@apple.com>
 
         Reviewed by Maciej.
diff --git a/LayoutTests/fast/dom/Window/window-resize-nan-expected.txt b/LayoutTests/fast/dom/Window/window-resize-nan-expected.txt
new file mode 100644 (file)
index 0000000..0302356
--- /dev/null
@@ -0,0 +1,61 @@
+This test makes sure that we cannot set the parameter of a window to NaN, causing unpredicable results
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+
+window.resizeTo Tests
+
+Testing - resizeTo: Bad width input
+PASS window.outerWidth is defaultSize
+PASS window.outerHeight is defaultSize
+
+Testing - resizeTo: Bad height input
+PASS window.outerWidth is defaultSize
+PASS window.outerHeight is defaultSize
+
+Testing - resizeTo: Bad width and height input
+PASS window.outerWidth is defaultSize
+PASS window.outerHeight is defaultSize
+
+window.resizeBy Tests
+
+Testing - resizeBy: Bad width input
+PASS window.outerWidth is defaultSize
+PASS window.outerHeight is defaultSize + y
+
+Testing - resizeBy: Bad height input
+PASS window.outerWidth is defaultSize + x
+PASS window.outerHeight is defaultSize
+
+window.moveTo Tests
+
+Testing - moveTo: Bad x input
+PASS window.screenY is y + (screen.availTop * 2)
+PASS window.screenX is screen.availLeft
+
+Testing - moveTo: Bad y input
+PASS window.screenY is screen.availTop * 2
+PASS window.screenX is x
+
+Testing - moveTo: Bad x and y input
+PASS window.screenY is screen.availTop * 2
+PASS window.screenX is screen.availLeft
+
+window.moveBy Tests
+
+Testing - moveBy: Bad x input
+PASS window.screenY is (screen.availTop * 4) + y
+PASS window.screenX is screen.availLeft
+
+Testing - moveBy: Bad y input
+PASS window.screenY is screen.availTop * 4
+PASS window.screenX is screen.availLeft + x
+
+Testing - moveBy: Bad x and y input
+PASS window.screenY is screen.availTop * 4
+PASS window.screenX is screen.availLeft
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/Window/window-resize-nan.html b/LayoutTests/fast/dom/Window/window-resize-nan.html
new file mode 100644 (file)
index 0000000..a43c548
--- /dev/null
@@ -0,0 +1,154 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+    <link rel="stylesheet" href="../../js/resources/js-test-style.css">
+    <script src="../../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script language="JavaScript" type="text/javascript">
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+
+    window.moveTo(0, 0);
+
+    description("This test makes sure that we cannot set the parameter of a \
+    window to NaN, causing unpredicable results");
+
+    var result;
+    var testName;
+    var x;
+    var y;
+    var badNum = "NaN";
+    var defaultSize = 500;
+    var defaultLocation = 0;
+
+    // initialize window to a known size
+    x = defaultSize;
+    y = defaultSize;
+    window.resizeTo(x, y);
+
+    // resizeTo /////////////////////////
+    debug('');
+    debug('window.resizeTo Tests');
+    debug('');
+
+    x = badNum;
+    y = defaultSize;
+    window.resizeTo(x, y);
+    debug("Testing - resizeTo: Bad width input");
+    shouldBe('window.outerWidth', 'defaultSize');
+    shouldBe('window.outerHeight', 'defaultSize');
+
+    debug('');
+
+    x = defaultSize;
+    y = badNum;
+    window.resizeTo(x, y);
+    debug("Testing - resizeTo: Bad height input");
+    shouldBe('window.outerWidth', 'defaultSize');
+    shouldBe('window.outerHeight', 'defaultSize');
+    
+    debug('');
+
+    x = badNum;
+    y = badNum;
+    window.resizeTo(x, y);
+    debug("Testing - resizeTo: Bad width and height input");
+    shouldBe('window.outerWidth', 'defaultSize');
+    shouldBe('window.outerHeight', 'defaultSize');
+
+    // resizeBy /////////////////////////
+    debug('');
+    debug('window.resizeBy Tests');
+    debug('');
+
+    x = badNum;
+    y = 100;
+    window.resizeBy(x, y);
+    debug("Testing - resizeBy: Bad width input");
+    shouldBe('window.outerWidth', 'defaultSize');
+    shouldBe('window.outerHeight', 'defaultSize + y');
+
+    debug('');
+    window.resizeTo(defaultSize, defaultSize);
+
+    x = 100;
+    y = badNum;
+    window.resizeBy(x, y);
+    debug("Testing - resizeBy: Bad height input");
+    shouldBe('window.outerWidth', 'defaultSize + x');
+    shouldBe('window.outerHeight', 'defaultSize');
+
+    // moveTo /////////////////////////
+    debug('');
+    debug('window.moveTo Tests');
+    debug('');
+    window.moveTo(screen.availLeft, screen.availTop);
+
+    x = badNum;
+    y = screen.availTop + 100;
+    window.moveTo(x, y);
+    debug("Testing - moveTo: Bad x input");
+    shouldBe('window.screenY', 'y + (screen.availTop * 2)'); // FIXME this should be just y
+    shouldBe('window.screenX', 'screen.availLeft');
+
+    debug('');
+    window.moveTo(screen.availLeft, screen.availTop);
+
+    x = screen.availLeft + 100;
+    y = badNum;
+    window.moveTo(x, y);
+    debug("Testing - moveTo: Bad y input");
+    shouldBe('window.screenY', 'screen.availTop * 2'); // FIXME this should just be screen.availTop
+    shouldBe('window.screenX', 'x');
+
+    debug('');
+    window.moveTo(screen.availLeft, screen.availTop);
+
+    x = badNum;
+    y = badNum;
+    window.moveTo(x, y);
+    debug("Testing - moveTo: Bad x and y input");
+    shouldBe('window.screenY', 'screen.availTop * 2'); // FIXME this should just be screen.availTop
+    shouldBe('window.screenX', 'screen.availLeft');
+
+    // moveBy /////////////////////////
+    debug('');
+    debug('window.moveBy Tests');
+    debug('');
+    window.moveTo(screen.availLeft, screen.availTop);
+
+    x = badNum;
+    y = 100;
+    window.moveBy(x, y);
+    debug("Testing - moveBy: Bad x input");
+    shouldBe('window.screenY', '(screen.availTop * 4) + y'); // FIXME this should just be screen.availTop + y
+    shouldBe('window.screenX', 'screen.availLeft');
+
+    debug('');
+    window.moveTo(screen.availLeft, screen.availTop);
+
+    x = 100;
+    y = badNum;
+    window.moveBy(x, y);
+    debug("Testing - moveBy: Bad y input");
+    shouldBe('window.screenY', 'screen.availTop * 4'); // FIXME this should just be screen.availTop
+    shouldBe('window.screenX', 'screen.availLeft + x');
+
+    debug('');
+    window.moveTo(screen.availLeft, screen.availTop);
+
+    x = badNum;
+    y = badNum;
+    window.moveBy(x, y);
+    debug("Testing - moveBy: Bad x and y input");
+    shouldBe('window.screenY', 'screen.availTop * 4'); // FIXME this should just be screen.availTop
+    shouldBe('window.screenX', 'screen.availLeft');
+
+    var successfullyParsed = true;
+</script>
+<script src="../../js/resources/js-test-post.js"></script>
+</body>
+</html>
index 7e5e0b62d5e065c3ca254f53b190593da38bcf56..60695330316758260ac01b442c11a54d040445d8 100644 (file)
@@ -1,3 +1,23 @@
+2007-10-26  Tristan O'Tierney  <tristan@apple.com>
+
+        Reviewed by Darin Adler.
+
+        <rdar://problem/5555053> REGRESSION:9A581: Window disappears when opening http://research.microsoft.com/users/darkok/
+        The problem was caused by checkin r24654. This change moved explicit bounds checking into adjustWindowRect
+        but failed to account for bounds checking (instead replaced with bounds clipping).  This caused issues
+        when NaN was used. This patch goes one step further and does NaN checking to prevent the possibility of
+        setting window bounds to NaN before an update occurs.
+
+        Test: fast/dom/Window/window-resize-nan.html
+        
+        * bindings/js/kjs_window.cpp:
+        (KJS::adjustWindowRect):
+        Added a new parameter, pendingChanges, which takes pending changes to the window
+        rect, and if they are valid (not NaN) sets them on window.
+        
+        (KJS::WindowFunc::callAsFunction):
+        Adjusted uses of adjustWindowRect to take new update parameter.
+
 2007-10-26  Sam Weinig  <sam@webkit.org>
 
         Reviewed by Tim Hatcher.
index cebe973f42990f1c4b54d57b7cad1c298b6d3aa6..a0324df32c362329cc9af8f60d9c39545230ce86 100644 (file)
@@ -1181,20 +1181,35 @@ static void parseWindowFeatures(const String& features, WindowFeatures& windowFe
 }
 
 // Explain the 4 things this function does.
-// 1) Constrains the window rect to no smaller than 100 in each dimension and no
+// 1) Validates the pending changes are not changing to NaN
+// 2) Constrains the window rect to no smaller than 100 in each dimension and no
 //    bigger than the the float rect's dimensions.
-// 2) Constrain window rect to within the top and left boundaries of the screen rect
-// 3) Constraint the window rect to within the bottom and right boundaries of the
+// 3) Constrain window rect to within the top and left boundaries of the screen rect
+// 4) Constraint the window rect to within the bottom and right boundaries of the
 //    screen rect.
-// 4) Translate the window rect coordinates to be within the coordinate space of
+// 5) Translate the window rect coordinates to be within the coordinate space of
 //    the screen rect.
-static void adjustWindowRect(const FloatRect& screen, FloatRect& window)
+static void adjustWindowRect(const FloatRect& screen, FloatRect& window, const FloatRect& pendingChanges)
 {
+    // Make sure we're in a valid state before adjusting dimensions
+    ASSERT(!isnan(screen.x()) && !isnan(screen.y()) && !isnan(screen.width()) && !isnan(screen.height()) &&
+           !isnan(window.x()) && !isnan(window.y()) && !isnan(window.width()) && !isnan(window.height()));
+    
+    // Update window values if they are not NaN
+    if (!isnan(pendingChanges.x()))
+        window.setX(pendingChanges.x());
+    if (!isnan(pendingChanges.y()))
+        window.setY(pendingChanges.y());
+    if (!isnan(pendingChanges.width()))
+        window.setWidth(pendingChanges.width());
+    if (!isnan(pendingChanges.height()))
+        window.setHeight(pendingChanges.height());
+    
     // Resize the window to between 100 and the screen width and height if it's
     // outside of those ranges.
     window.setWidth(min(max(100.0f, window.width()), screen.width()));
     window.setHeight(min(max(100.0f, window.height()), screen.height()));
-
+    
     // Constrain the window to the top and left of the screen if it's left or
     // above it.
     window.setX(max(window.x(), screen.x()));
@@ -1289,7 +1304,7 @@ JSValue *WindowFunc::callAsFunction(ExecState *exec, JSObject *thisObj, const Li
       String features = valueToStringWithUndefinedOrNullCheck(exec, args[2]);
       parseWindowFeatures(features, windowFeatures);
       FloatRect windowRect(windowFeatures.x, windowFeatures.y, windowFeatures.width, windowFeatures.height);
-      adjustWindowRect(screenAvailableRect(page->mainFrame()->view()), windowRect);
+      adjustWindowRect(screenAvailableRect(page->mainFrame()->view()), windowRect, windowRect);
 
       windowFeatures.x = windowRect.x();
       windowFeatures.y = windowRect.y();
@@ -1317,9 +1332,10 @@ JSValue *WindowFunc::callAsFunction(ExecState *exec, JSObject *thisObj, const Li
   case Window::MoveBy:
     if (args.size() >= 2 && page) {
       FloatRect fr = page->chrome()->windowRect();
-      fr.move(args[0]->toFloat(exec), args[1]->toFloat(exec));
+      FloatRect update = fr;
+      update.move(args[0]->toFloat(exec), args[1]->toFloat(exec));
       // Security check (the spec talks about UniversalBrowserWrite to disable this check...)
-      adjustWindowRect(screenAvailableRect(page->mainFrame()->view()), fr);
+      adjustWindowRect(screenAvailableRect(page->mainFrame()->view()), fr, update);
       page->chrome()->setWindowRect(fr);
     }
     return jsUndefined();
@@ -1328,27 +1344,28 @@ JSValue *WindowFunc::callAsFunction(ExecState *exec, JSObject *thisObj, const Li
       FloatRect fr = page->chrome()->windowRect();
       FloatRect sr = screenAvailableRect(page->mainFrame()->view());
       fr.setLocation(sr.location());
-      fr.move(args[0]->toFloat(exec), args[1]->toFloat(exec));
+      FloatRect update = fr;
+      update.move(args[0]->toFloat(exec), args[1]->toFloat(exec));     
       // Security check (the spec talks about UniversalBrowserWrite to disable this check...)
-      adjustWindowRect(sr, fr);
+      adjustWindowRect(sr, fr, update);
       page->chrome()->setWindowRect(fr);
     }
     return jsUndefined();
   case Window::ResizeBy:
     if (args.size() >= 2 && page) {
-      FloatRect r = page->chrome()->windowRect();
-      FloatSize dest = r.size() + FloatSize(args[0]->toFloat(exec), args[1]->toFloat(exec));
-      FloatRect fr = FloatRect(r.location(), dest);
-      adjustWindowRect(screenAvailableRect(page->mainFrame()->view()), fr);
+      FloatRect fr = page->chrome()->windowRect();
+      FloatSize dest = fr.size() + FloatSize(args[0]->toFloat(exec), args[1]->toFloat(exec));
+      FloatRect update(fr.location(), dest);
+      adjustWindowRect(screenAvailableRect(page->mainFrame()->view()), fr, update);
       page->chrome()->setWindowRect(fr);
     }
     return jsUndefined();
   case Window::ResizeTo:
     if (args.size() >= 2 && page) {
-      FloatRect r = page->chrome()->windowRect();
+      FloatRect fr = page->chrome()->windowRect();
       FloatSize dest = FloatSize(args[0]->toFloat(exec), args[1]->toFloat(exec));
-      FloatRect fr = FloatRect(r.location(), dest);
-      adjustWindowRect(screenAvailableRect(page->mainFrame()->view()), fr);
+      FloatRect update(fr.location(), dest);
+      adjustWindowRect(screenAvailableRect(page->mainFrame()->view()), fr, update);
       page->chrome()->setWindowRect(fr);
     }
     return jsUndefined();