document.open() should not propagate URLs to non-fully active documents
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 27 Sep 2018 16:31:53 +0000 (16:31 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 27 Sep 2018 16:31:53 +0000 (16:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=189375
<rdar://problem/44282755>

Reviewed by Youenn Fablet.

LayoutTests/imported/w3c:

Rebaseline WPT test now that more checks are passing.

* web-platform-tests/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/url.window-expected.txt:

Source/WebCore:

Update our document.open() to not propagate URLs to non-fully active documents, as per:
- https://html.spec.whatwg.org/#document-open-steps (Step 11)

A "fully active" document is defined by at:
- https://html.spec.whatwg.org/#fully-active

No new tests, rebaselined existing test.

* dom/Document.cpp:
(WebCore::Document::open):
(WebCore::Document::isFullyActive const):
* dom/Document.h:
* dom/Document.idl:

LayoutTests:

Update existing test to reflect behavior change. I have verified that this test was
failing in Firefox and is now passing in Firefox.

* fast/dom/resource-locations-in-created-html-document.html:

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/resource-locations-in-created-html-document.html
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/url.window-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/dom/Document.idl

index df326b0..fe782e4 100644 (file)
@@ -1,5 +1,18 @@
 2018-09-27  Chris Dumez  <cdumez@apple.com>
 
+        document.open() should not propagate URLs to non-fully active documents
+        https://bugs.webkit.org/show_bug.cgi?id=189375
+        <rdar://problem/44282755>
+
+        Reviewed by Youenn Fablet.
+
+        Update existing test to reflect behavior change. I have verified that this test was
+        failing in Firefox and is now passing in Firefox.
+
+        * fast/dom/resource-locations-in-created-html-document.html:
+
+2018-09-27  Chris Dumez  <cdumez@apple.com>
+
         The WebContent process should not process incoming IPC while waiting for a sync IPC reply
         https://bugs.webkit.org/show_bug.cgi?id=184183
         <rdar://problem/36800576>
index 4dc08d4..891e2bd 100644 (file)
@@ -12,9 +12,7 @@
 
         var path = htmlDoc.getElementById('theImage').src;
 
-        if (path == 'file:///test')
-            document.getElementById('result').innerHTML = 'SUCCESS';
-        else if (/^file:\/\/\/[C-Z]:\/test$/.test(path))  // MS Windows.
+        if (path === '/test')
             document.getElementById('result').innerHTML = 'SUCCESS';
     }
     </script>
index 9a88a1e..b5a62b7 100644 (file)
@@ -1,3 +1,15 @@
+2018-09-27  Chris Dumez  <cdumez@apple.com>
+
+        document.open() should not propagate URLs to non-fully active documents
+        https://bugs.webkit.org/show_bug.cgi?id=189375
+        <rdar://problem/44282755>
+
+        Reviewed by Youenn Fablet.
+
+        Rebaseline WPT test now that more checks are passing.
+
+        * web-platform-tests/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/url.window-expected.txt:
+
 2018-09-27  Antoine Quint  <graouts@apple.com>
 
         [Web Animations] Turn Web Animations with CSS integration on
index d1c6c75..cf60bad 100644 (file)
@@ -1,7 +1,7 @@
 
 PASS document.open() changes document's URL (fully active document) 
-FAIL document.open() does not change document's URL (active but not fully active document) assert_equals: expected "http://localhost:8800/common/blank.html" but got "http://localhost:8800/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/url.window.html"
-FAIL document.open() does not change document's URL (non-active document with an associated Window object; frame is removed) assert_equals: expected "about:blank" but got "http://localhost:8800/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/url.window.html"
-FAIL document.open() does not change document's URL (non-active document with an associated Window object; navigated away) assert_equals: expected "about:blank" but got "http://localhost:8800/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/url.window.html"
-FAIL document.open() does not change document's URL (non-active document without an associated Window object) assert_equals: expected "about:blank" but got "http://localhost:8800/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/url.window.html"
+FAIL document.open() does not change document's URL (active but not fully active document) null is not an object (evaluating 'childWin.location.href')
+PASS document.open() does not change document's URL (non-active document with an associated Window object; frame is removed) 
+PASS document.open() does not change document's URL (non-active document with an associated Window object; navigated away) 
+PASS document.open() does not change document's URL (non-active document without an associated Window object) 
 
index b577883..d190799 100644 (file)
@@ -1,3 +1,25 @@
+2018-09-27  Chris Dumez  <cdumez@apple.com>
+
+        document.open() should not propagate URLs to non-fully active documents
+        https://bugs.webkit.org/show_bug.cgi?id=189375
+        <rdar://problem/44282755>
+
+        Reviewed by Youenn Fablet.
+
+        Update our document.open() to not propagate URLs to non-fully active documents, as per:
+        - https://html.spec.whatwg.org/#document-open-steps (Step 11)
+
+        A "fully active" document is defined by at:
+        - https://html.spec.whatwg.org/#fully-active
+
+        No new tests, rebaselined existing test.
+
+        * dom/Document.cpp:
+        (WebCore::Document::open):
+        (WebCore::Document::isFullyActive const):
+        * dom/Document.h:
+        * dom/Document.idl:
+
 2018-09-27  Alicia Boya GarcĂ­a  <aboya@igalia.com>
 
         [MSE][GStreamer] Use sentinel buffer to detect end of append
index 10b6d28..23bd5af 100644 (file)
@@ -2661,7 +2661,6 @@ ExceptionOr<RefPtr<WindowProxy>> Document::openForBindings(DOMWindow& activeWind
     return m_domWindow->open(activeWindow, firstWindow, url, name, features);
 }
 
-// FIXME: Add support for the 'type' and 'replace' parameters.
 ExceptionOr<Document&> Document::openForBindings(Document* responsibleDocument, const String&, const String&)
 {
     if (!isHTMLDocument() || m_throwOnDynamicMarkupInsertionCount)
@@ -2696,7 +2695,7 @@ void Document::open(Document* responsibleDocument)
 
     removeAllEventListeners();
 
-    if (responsibleDocument) {
+    if (responsibleDocument && isFullyActive()) {
         setURL(responsibleDocument->url());
         setCookieURL(responsibleDocument->cookieURL());
         setSecurityOriginPolicy(responsibleDocument->securityOriginPolicy());
@@ -2710,6 +2709,20 @@ void Document::open(Document* responsibleDocument)
         m_frame->loader().didExplicitOpen();
 }
 
+// https://html.spec.whatwg.org/#fully-active
+bool Document::isFullyActive() const
+{
+    auto* frame = this->frame();
+    if (!frame || frame->document() != this)
+        return false;
+
+    if (frame->isMainFrame())
+        return true;
+
+    auto* parentFrame = frame->tree().parent();
+    return parentFrame && parentFrame->document() && parentFrame->document()->isFullyActive();
+}
+
 void Document::detachParser()
 {
     if (!m_parser)
index b09577d..a17b8cd 100644 (file)
@@ -644,7 +644,7 @@ public:
     WEBCORE_EXPORT DocumentLoader* loader() const;
 
     WEBCORE_EXPORT ExceptionOr<RefPtr<WindowProxy>> openForBindings(DOMWindow& activeWindow, DOMWindow& firstDOMWindow, const String& url, const AtomicString& name, const String& features);
-    WEBCORE_EXPORT ExceptionOr<Document&> openForBindings(Document* responsibleDocument, const String& type, const String& replace);
+    WEBCORE_EXPORT ExceptionOr<Document&> openForBindings(Document* responsibleDocument, const String&, const String&);
 
     // FIXME: We should rename this at some point and give back the name 'open' to the HTML specified ones.
     WEBCORE_EXPORT void open(Document* responsibleDocument = nullptr);
@@ -943,6 +943,8 @@ public:
     const URL& firstPartyForCookies() const { return m_firstPartyForCookies; }
     void setFirstPartyForCookies(const URL& url) { m_firstPartyForCookies = url; }
 
+    bool isFullyActive() const;
+
     // The full URL corresponding to the "site for cookies" in the Same-Site Cookies spec.,
     // <https://tools.ietf.org/html/draft-ietf-httpbis-cookie-same-site-00>. It is either
     // the URL of the top-level document or the null URL depending on whether the registrable
index d54d8c7..426a294 100644 (file)
@@ -107,7 +107,7 @@ typedef (
     // FIXME: The HTML spec says this should consult the "responsible document". We should ensure
     // that the caller document matches those semantics. It is possible we should replace it with
     // the existing 'incumbent document' concept.
-    [CEReactions, CallWith=ResponsibleDocument, ImplementedAs=openForBindings, MayThrowException] Document open(optional DOMString type = "text/html", optional DOMString replace = "");
+    [CEReactions, CallWith=ResponsibleDocument, ImplementedAs=openForBindings, MayThrowException] Document open(optional DOMString unused1, optional DOMString unused2); // both arguments are ignored.
     [CallWith=ActiveWindow&FirstWindow, ImplementedAs=openForBindings, MayThrowException] WindowProxy open(USVString url, DOMString name, DOMString features);
     [CEReactions, ImplementedAs=closeForBindings, MayThrowException] void close();
     [CEReactions, CallWith=ResponsibleDocument, MayThrowException] void write(DOMString... text);