WebKitTestRunner: Add comment to TestRunner::setCanOpenWindows()
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 21 Dec 2016 19:35:52 +0000 (19:35 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 21 Dec 2016 19:35:52 +0000 (19:35 +0000)
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:

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

LayoutTests/ChangeLog
LayoutTests/platform/wk2/TestExpectations
Tools/ChangeLog
Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp

index 11bc20e..ce52261 100644 (file)
@@ -1,3 +1,13 @@
+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>
 
         Web Inspector: Fix some typos and style in LayoutTests/inspector
index 93da7ef..1a7e69d 100644 (file)
@@ -315,7 +315,8 @@ plugins/plugin-document-willSendRequest-null.html [ Skip ]
 plugins/plugin-remove-readystatechange.html [ Skip ]
 
 # 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
 
 # requires video.buffered to be able to return multiple timeranges
index 93a2e57..472b8ad 100644 (file)
@@ -1,3 +1,20 @@
+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>
 
         REGRESSION: API test failure: WKWebView.EvaluateJavaScriptBlockCrash
index 1413123..b3bf995 100644 (file)
@@ -329,8 +329,13 @@ bool TestRunner::isCommandEnabled(JSStringRef name)
 
 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.
 }
 
 void TestRunner::setXSSAuditorEnabled(bool enabled)