[webkit-reviews] review denied: [Bug 36721] Use Chromium's plugins/get-url-with-blank-target.html for all platforms : [Attachment 53051] Updated Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Robert Hogan
<robert at webkit.org>'s request for review:
Bug 36721: Use Chromium's plugins/get-url-with-blank-target.html for all
platforms
https://bugs.webkit.org/show_bug.cgi?id=36721

Attachment 53051: Updated Patch
https://bugs.webkit.org/attachment.cgi?id=53051&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
> Chromium introduced a new version of test that works for Qt and possibly
other platforms.

What prevents the original test from working on other platforms?

-    if (result == NPERR_GENERIC_ERROR)
+    if (result == NPERR_NO_ERROR)
	 d.innerHTML = "SUCCESS"

> The Mac result expects an NP_ERROR_GENERIC whereas 
> Chromium/Qt expect NP_NO_ERROR, which looking at PluginView.cpp
> is how it should be:

I don't understand the logic here. Why does implementation tell us which
behavior is correct?

+	 layoutTestController.setCanOpenWindows();
...
+This tests that we won't crash when a plugin tries to open an URL in a new
window when the application does not create the window. If this test is
successful, the word SUCCESS should be seen below.

The description is correct. This test verifies what happens when the window
fails to open, so the test shouldn't call setCanOpenWindows().