Title: [196706] trunk
Revision
196706
Author
cdu...@apple.com
Date
2016-02-17 11:31:10 -0800 (Wed, 17 Feb 2016)

Log Message

Regression(r196648): window.showModalDialog is no longer undefined if the client does not allow showing modal dialog
https://bugs.webkit.org/show_bug.cgi?id=154330

Reviewed by Gavin Barraclough.

Source/WebCore:

window.showModalDialog is no longer undefined if the client does not
allow showing modal dialog after r196648. This patch fixes the issue
and add test coverage for this.

Test: fast/dom/Window/forbid-showModalDialog.html

* bindings/js/JSDOMWindowCustom.cpp:
(WebCore::JSDOMWindow::getOwnPropertySlot):
- Move the DOMWindow::canShowModalDialog() check *before* checking
  for static properties as showModalDialog is now in the static
  property table after r196648.
- Add check for Base::getOwnPropertySlot() first to support overriding
  window.showModalDialog (This behavior matches Firefox).
- Return false if DOMWindow::canShowModalDialog() returns false as this
  seems cleaner than claiming that the property is there but undefined.

* page/DOMWindow.cpp:
(WebCore::DOMWindow::canShowModalDialogNow): Deleted.
This was indentical to canShowModalDialog().

(WebCore::DOMWindow::canShowModalDialog):
(WebCore::DOMWindow::setCanShowModalDialogOverride):
(WebCore::DOMWindow::showModalDialog):
* page/DOMWindow.h:
* testing/Internals.cpp:
(WebCore::Internals::setCanShowModalDialogOverride):
* testing/Internals.h:
* testing/Internals.idl:
Add support for overriding the ChromeClient's canShowModalDialog
decision and hook it up to Internals to add layout test coverage.

LayoutTests:

Add layout test to make sure that window.showModalDialog is undefined
when the client does not allow showing modal dialog and to check that
window.showModalDialog can be shadowed.

* fast/dom/Window/forbid-showModalDialog-expected.txt: Added.
* fast/dom/Window/forbid-showModalDialog.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (196705 => 196706)


--- trunk/LayoutTests/ChangeLog	2016-02-17 19:21:59 UTC (rev 196705)
+++ trunk/LayoutTests/ChangeLog	2016-02-17 19:31:10 UTC (rev 196706)
@@ -1,3 +1,17 @@
+2016-02-17  Chris Dumez  <cdu...@apple.com>
+
+        Regression(r196648): window.showModalDialog is no longer undefined if the client does not allow showing modal dialog
+        https://bugs.webkit.org/show_bug.cgi?id=154330
+
+        Reviewed by Gavin Barraclough.
+
+        Add layout test to make sure that window.showModalDialog is undefined
+        when the client does not allow showing modal dialog and to check that
+        window.showModalDialog can be shadowed.
+
+        * fast/dom/Window/forbid-showModalDialog-expected.txt: Added.
+        * fast/dom/Window/forbid-showModalDialog.html: Added.
+
 2016-02-17  Commit Queue  <commit-qu...@webkit.org>
 
         Unreviewed, rolling out r196675.

Added: trunk/LayoutTests/fast/dom/Window/forbid-showModalDialog-expected.txt (0 => 196706)


--- trunk/LayoutTests/fast/dom/Window/forbid-showModalDialog-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/Window/forbid-showModalDialog-expected.txt	2016-02-17 19:31:10 UTC (rev 196706)
@@ -0,0 +1,31 @@
+Tests that Window.showModalDialog is undefined if the client does not allow showing modal dialogs.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Make sure window.showModalDialog is undefined when modal dialogs are not allowed.
+internals.setCanShowModalDialogOverride(false)
+PASS window.showModalDialog is undefined
+PASS window.hasOwnProperty('showModalDialog') is false
+
+Tests having a named property with name 'showModalDialog'.
+document.body.append(testFrame)
+PASS window.showModalDialog is testFrame.contentWindow
+testFrame.remove()
+PASS window.showModalDialog is undefined
+
+Tests that window.showModalDialog is no longer undefined when modal dialogs become allowed.
+internals.setCanShowModalDialogOverride(true)
+PASS window.showModalDialog is not undefined
+PASS window.hasOwnProperty('showModalDialog') is true
+PASS window.showModalDialog is an instance of Function
+
+Make sure window.showModalDialog can be shadowed.
+window.showModalDialog = 1
+PASS window.showModalDialog is 1
+delete window.showModalDialog
+PASS window.showModalDialog is undefined
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/dom/Window/forbid-showModalDialog.html (0 => 196706)


--- trunk/LayoutTests/fast/dom/Window/forbid-showModalDialog.html	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/Window/forbid-showModalDialog.html	2016-02-17 19:31:10 UTC (rev 196706)
@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description("Tests that Window.showModalDialog is undefined if the client does not allow showing modal dialogs.");
+
+debug("Make sure window.showModalDialog is undefined when modal dialogs are not allowed.");
+evalAndLog("internals.setCanShowModalDialogOverride(false)");
+shouldBe("window.showModalDialog", "undefined");
+shouldBeFalse("window.hasOwnProperty('showModalDialog')");
+
+
+debug("");
+debug("Tests having a named property with name 'showModalDialog'.");
+var testFrame = document.createElement("iframe");
+testFrame.name = "showModalDialog";
+evalAndLog("document.body.append(testFrame)");
+shouldBe("window.showModalDialog", "testFrame.contentWindow");
+evalAndLog("testFrame.remove()");
+shouldBe("window.showModalDialog", "undefined");
+
+debug("");
+debug("Tests that window.showModalDialog is no longer undefined when modal dialogs become allowed.");
+evalAndLog("internals.setCanShowModalDialogOverride(true)");
+shouldNotBe("window.showModalDialog", "undefined");
+shouldBeTrue("window.hasOwnProperty('showModalDialog')");
+shouldBeType("window.showModalDialog", "Function");
+
+debug("");
+debug("Make sure window.showModalDialog can be shadowed.");
+evalAndLog("window.showModalDialog = 1");
+shouldBe("window.showModalDialog" , "1");
+evalAndLog("delete window.showModalDialog");
+shouldBe("window.showModalDialog", "undefined");
+
+</script>
+<script src=""
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (196705 => 196706)


--- trunk/Source/WebCore/ChangeLog	2016-02-17 19:21:59 UTC (rev 196705)
+++ trunk/Source/WebCore/ChangeLog	2016-02-17 19:31:10 UTC (rev 196706)
@@ -1,3 +1,41 @@
+2016-02-17  Chris Dumez  <cdu...@apple.com>
+
+        Regression(r196648): window.showModalDialog is no longer undefined if the client does not allow showing modal dialog
+        https://bugs.webkit.org/show_bug.cgi?id=154330
+
+        Reviewed by Gavin Barraclough.
+
+        window.showModalDialog is no longer undefined if the client does not
+        allow showing modal dialog after r196648. This patch fixes the issue
+        and add test coverage for this.
+
+        Test: fast/dom/Window/forbid-showModalDialog.html
+
+        * bindings/js/JSDOMWindowCustom.cpp:
+        (WebCore::JSDOMWindow::getOwnPropertySlot):
+        - Move the DOMWindow::canShowModalDialog() check *before* checking
+          for static properties as showModalDialog is now in the static
+          property table after r196648.
+        - Add check for Base::getOwnPropertySlot() first to support overriding
+          window.showModalDialog (This behavior matches Firefox).
+        - Return false if DOMWindow::canShowModalDialog() returns false as this
+          seems cleaner than claiming that the property is there but undefined.
+
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::canShowModalDialogNow): Deleted.
+        This was indentical to canShowModalDialog().
+
+        (WebCore::DOMWindow::canShowModalDialog):
+        (WebCore::DOMWindow::setCanShowModalDialogOverride):
+        (WebCore::DOMWindow::showModalDialog):
+        * page/DOMWindow.h:
+        * testing/Internals.cpp:
+        (WebCore::Internals::setCanShowModalDialogOverride):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+        Add support for overriding the ChromeClient's canShowModalDialog
+        decision and hook it up to Internals to add layout test coverage.
+
 2016-02-17  Brady Eidson  <beid...@apple.com>
 
         Modern IDB: More WK2 IPC Scaffolding.

Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp (196705 => 196706)


--- trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp	2016-02-17 19:21:59 UTC (rev 196705)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp	2016-02-17 19:31:10 UTC (rev 196706)
@@ -275,16 +275,18 @@
             slot.setCustom(thisObject, ReadOnly | DontDelete | DontEnum, nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionPostMessage, 2>);
         return true;
     }
+
+    if (propertyName == exec->propertyNames().showModalDialog) {
+        if (Base::getOwnPropertySlot(thisObject, exec, propertyName, slot))
+            return true;
+        if (!DOMWindow::canShowModalDialog(frame))
+            return jsDOMWindowGetOwnPropertySlotNamedItemGetter(thisObject, *frame, exec, propertyName, slot);
+    }
+
     // (2) Regular own properties.
     if (getStaticPropertySlot<JSDOMWindow, Base>(exec, *JSDOMWindow::info()->staticPropHashTable, thisObject, propertyName, slot))
         return true;
-    // FIXME: this looks pretty bogus. It seems highly likely that if !canShowModalDialog the
-    // funtion should still be present, or should be omitted entirely - present but reads as
-    // undefined with unspecified attributes is likely wrong.
-    if (propertyName == exec->propertyNames().showModalDialog && !DOMWindow::canShowModalDialog(frame)) {
-        slot.setUndefined();
-        return true;
-    }
+
 #if ENABLE(INDEXED_DATABASE)
     // FIXME: With generated JS bindings built on static property tables there is no way to
     // completely remove a generated property at runtime. So to completely disable IndexedDB

Modified: trunk/Source/WebCore/page/DOMWindow.cpp (196705 => 196706)


--- trunk/Source/WebCore/page/DOMWindow.cpp	2016-02-17 19:21:59 UTC (rev 196705)
+++ trunk/Source/WebCore/page/DOMWindow.cpp	2016-02-17 19:31:10 UTC (rev 196706)
@@ -373,20 +373,22 @@
 {
     if (!frame)
         return false;
-    Page* page = frame->page();
-    if (!page)
-        return false;
-    return page->chrome().canRunModal();
+
+    // Override support for layout testing purposes.
+    if (auto* document = frame->document()) {
+        if (auto* window = document->domWindow()) {
+            if (window->m_canShowModalDialogOverride)
+                return window->m_canShowModalDialogOverride.value();
+        }
+    }
+
+    auto* page = frame->page();
+    return page ? page->chrome().canRunModal() : false;
 }
 
-bool DOMWindow::canShowModalDialogNow(const Frame* frame)
+void DOMWindow::setCanShowModalDialogOverride(bool allow)
 {
-    if (!frame)
-        return false;
-    Page* page = frame->page();
-    if (!page)
-        return false;
-    return page->chrome().canRunModal();
+    m_canShowModalDialogOverride = allow;
 }
 
 DOMWindow::DOMWindow(Document* document)
@@ -2240,7 +2242,7 @@
         return;
     }
 
-    if (!canShowModalDialogNow(m_frame) || !firstWindow.allowPopUp())
+    if (!canShowModalDialog(m_frame) || !firstWindow.allowPopUp())
         return;
 
     RefPtr<Frame> dialogFrame = createWindow(urlString, emptyAtom, parseDialogFeatures(dialogFeaturesString, screenAvailableRect(m_frame->view())), activeWindow, *firstFrame, *m_frame, WTFMove(prepareDialogFunction));

Modified: trunk/Source/WebCore/page/DOMWindow.h (196705 => 196706)


--- trunk/Source/WebCore/page/DOMWindow.h	2016-02-17 19:21:59 UTC (rev 196705)
+++ trunk/Source/WebCore/page/DOMWindow.h	2016-02-17 19:31:10 UTC (rev 196706)
@@ -34,6 +34,7 @@
 #include "Supplementable.h"
 #include <functional>
 #include <memory>
+#include <wtf/Optional.h>
 #include <wtf/WeakPtr.h>
 
 namespace Inspector {
@@ -135,7 +136,7 @@
         bool allowPopUp(); // Call on first window, not target window.
         static bool allowPopUp(Frame* firstFrame);
         static bool canShowModalDialog(const Frame*);
-        static bool canShowModalDialogNow(const Frame*);
+        WEBCORE_EXPORT void setCanShowModalDialogOverride(bool);
 
         // DOM Level 0
 
@@ -370,6 +371,7 @@
 
         bool m_shouldPrintWhenFinishedLoading;
         bool m_suspendedForDocumentSuspension;
+        Optional<bool> m_canShowModalDialogOverride;
 
         HashSet<DOMWindowProperty*> m_properties;
 

Modified: trunk/Source/WebCore/testing/Internals.cpp (196705 => 196706)


--- trunk/Source/WebCore/testing/Internals.cpp	2016-02-17 19:21:59 UTC (rev 196705)
+++ trunk/Source/WebCore/testing/Internals.cpp	2016-02-17 19:31:10 UTC (rev 196706)
@@ -580,6 +580,16 @@
     frame()->loader().setOverrideCachePolicyForTesting(stringToResourceRequestCachePolicy(policy));
 }
 
+void Internals::setCanShowModalDialogOverride(bool allow, ExceptionCode& ec)
+{
+    if (!contextDocument() || !contextDocument()->domWindow()) {
+        ec = INVALID_ACCESS_ERR;
+        return;
+    }
+
+    contextDocument()->domWindow()->setCanShowModalDialogOverride(allow);
+}
+
 static ResourceLoadPriority stringToResourceLoadPriority(const String& policy)
 {
     if (policy == "ResourceLoadPriorityVeryLow")

Modified: trunk/Source/WebCore/testing/Internals.h (196705 => 196706)


--- trunk/Source/WebCore/testing/Internals.h	2016-02-17 19:21:59 UTC (rev 196705)
+++ trunk/Source/WebCore/testing/Internals.h	2016-02-17 19:31:10 UTC (rev 196706)
@@ -98,6 +98,7 @@
     bool isSharingStyleSheetContents(Element* linkA, Element* linkB);
     bool isStyleSheetLoadingSubresources(Element* link);
     void setOverrideCachePolicy(const String&);
+    void setCanShowModalDialogOverride(bool allow, ExceptionCode&);
     void setOverrideResourceLoadPriority(const String&);
     void setStrictRawResourceValidationPolicyDisabled(bool);
 

Modified: trunk/Source/WebCore/testing/Internals.idl (196705 => 196706)


--- trunk/Source/WebCore/testing/Internals.idl	2016-02-17 19:21:59 UTC (rev 196705)
+++ trunk/Source/WebCore/testing/Internals.idl	2016-02-17 19:31:10 UTC (rev 196706)
@@ -437,4 +437,6 @@
 
     DOMString resourceLoadStatisticsForOrigin(DOMString domain);
     void setResourceLoadStatisticsEnabled(boolean enable);
+
+    [RaisesException] void setCanShowModalDialogOverride(boolean allow);
 };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to