Changeset 210070 in webkit

Timestamp:

Dec 21, 2016, 11:35:52 AM (9 years ago)

Author:

dbates@webkit.org

Message:

WebKitTestRunner: Add comment to TestRunner::setCanOpenWindows()
​https://bugs.webkit.org/show_bug.cgi?id=166356

Tools:

Add comment in TestRunner::setCanOpenWindows() to elaborate further on its purpose
as a means to allow a test to explicit allow an embedding client to open a new windows
such that the default behavior is for the embedding client to forbid such an action.
The test plugins/get-url-with-blank-target.html assumes this default behavior though
it is currently skipped on WebKit2 because questions were raised in the patch for
<​https://bugs.webkit.org/show_bug.cgi?id=43389> about its correctness and usefulness.
We will need to implement TestRunner::setCanOpenWindows() should we decide to unskip
this test.

• WebKitTestRunner/InjectedBundle/TestRunner.cpp:

(WTR::TestRunner::setCanOpenWindows):

LayoutTests:

Add a remark that the test plugins/get-url-with-blank-target.html depends on
the assumption that WebKitTestRunner forbids opening windows by default.

• platform/wk2/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/wk2/TestExpectations (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-12-21  Daniel Bates  <dabates@apple.com>
+
+        WebKitTestRunner: Add comment to TestRunner::setCanOpenWindows()
+        https://bugs.webkit.org/show_bug.cgi?id=166356
+
+        Add a remark that the test plugins/get-url-with-blank-target.html depends on
+        the assumption that WebKitTestRunner forbids opening windows by default.
+
+        * platform/wk2/TestExpectations:
+
 2016-12-21  Joseph Pecoraro  <pecoraro@apple.com>
 

trunk/LayoutTests/platform/wk2/TestExpectations

@@ -316 +316 @@
 
 # This test checks that NPN_GetURL with a blank target will return an error if the window isn't opened. This behavior doesn't
-# match Firefox or Chrome. We should either fix the test, or get rid of it.
+# match Firefox or Chrome. We should either fix the test, or get rid of it. This test also depends on implementing
+# testRunner::setCanOpenWindows() such that opening windows are forbidden by default.
 plugins/get-url-with-blank-target.html
 

trunk/Tools/ChangeLog

@@ +1 @@
+2016-12-21  Daniel Bates  <dabates@apple.com>
+
+        WebKitTestRunner: Add comment to TestRunner::setCanOpenWindows()
+        https://bugs.webkit.org/show_bug.cgi?id=166356
+
+        Add comment in TestRunner::setCanOpenWindows() to elaborate further on its purpose
+        as a means to allow a test to explicit allow an embedding client to open a new windows
+        such that the default behavior is for the embedding client to forbid such an action.
+        The test plugins/get-url-with-blank-target.html assumes this default behavior though
+        it is currently skipped on WebKit2 because questions were raised in the patch for
+        <https://bugs.webkit.org/show_bug.cgi?id=43389> about its correctness and usefulness.
+        We will need to implement TestRunner::setCanOpenWindows() should we decide to unskip
+        this test.
+
+        * WebKitTestRunner/InjectedBundle/TestRunner.cpp:
+        (WTR::TestRunner::setCanOpenWindows):
+
 2016-12-20  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp

@@ -330 +330 @@
 void TestRunner::setCanOpenWindows(bool)
 {
-    // It's not clear if or why any tests require opening windows be forbidden.
-    // For now, just ignore this setting, and if we find later it's needed we can add it.
+    // The test plugins/get-url-with-blank-target.html requires that the embedding client forbid
+    // opening windows (by omitting a call to this function) so as to test that NPN_GetURL()
+    // with a blank target will return an error.
+    //
+    // It is not clear if we should implement this functionality or remove it and plugins/get-url-with-blank-target.html
+    // per the remark in <https://trac.webkit.org/changeset/64504/trunk/LayoutTests/platform/mac-wk2/Skipped>.
+    // For now, just ignore this setting.
 }