5 files changed, 14 insertions, 18 deletions
diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs
index 3606ffc7c9d..eacd6a11745 100644
--- a/components/script/dom/document.rs
+++ b/components/script/dom/document.rs
@@ -4685,14 +4685,10 @@ impl DocumentMethods for Document {
         url: USVString,
         target: DOMString,
         features: DOMString,
-    ) -> Fallible<DomRoot<WindowProxy>> {
-        // WhatWG spec states this should always return a WindowProxy, but the spec for WindowProxy.open states
-        // it optionally returns a WindowProxy. Assume an error if window.open returns none.
-        // See https://github.com/whatwg/html/issues/4091
-        let context = self.browsing_context().ok_or(Error::InvalidAccess)?;
-        context
+    ) -> Fallible<Option<DomRoot<WindowProxy>>> {
+        self.browsing_context()
+            .ok_or(Error::InvalidAccess)?
             .open(url, target, features)
-            .ok_or(Error::InvalidAccess)
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-document-write
diff --git a/components/script/dom/webidls/Document.webidl b/components/script/dom/webidls/Document.webidl
index a9fe7fc982b..8ca4bbe50a1 100644
--- a/components/script/dom/webidls/Document.webidl
+++ b/components/script/dom/webidls/Document.webidl
@@ -124,7 +124,7 @@ partial /*sealed*/ interface Document {
   [CEReactions, Throws]
   Document open(optional DOMString unused1, optional DOMString unused2);
   [CEReactions, Throws]
-  WindowProxy open(USVString url, DOMString name, DOMString features);
+  WindowProxy? open(USVString url, DOMString name, DOMString features);
   [CEReactions, Throws]
   void close();
   [CEReactions, Throws]
diff --git a/components/script/dom/webidls/Window.webidl b/components/script/dom/webidls/Window.webidl
index 0c0b5e80860..f0b5b47d7af 100644
--- a/components/script/dom/webidls/Window.webidl
+++ b/components/script/dom/webidls/Window.webidl
@@ -40,8 +40,8 @@
   // https://github.com/whatwg/html/issues/2115
   [Replaceable] readonly attribute WindowProxy? parent;
   readonly attribute Element? frameElement;
-  WindowProxy? open(optional USVString url = "", optional DOMString target = "_blank",
-                    optional DOMString features = "");
+  [Throws] WindowProxy? open(optional USVString url = "", optional DOMString target = "_blank",
+                             optional DOMString features = "");
   //getter WindowProxy (unsigned long index);
 
   // https://github.com/servo/servo/issues/14453
diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs
index ccb4b8dfb48..93019cf913d 100644
--- a/components/script/dom/window.rs
+++ b/components/script/dom/window.rs
@@ -661,7 +661,7 @@ impl WindowMethods for Window {
         url: USVString,
         target: DOMString,
         features: DOMString,
-    ) -> Option<DomRoot<WindowProxy>> {
+    ) -> Fallible<Option<DomRoot<WindowProxy>>> {
         self.window_proxy().open(url, target, features)
     }
 
diff --git a/components/script/dom/windowproxy.rs b/components/script/dom/windowproxy.rs
index 4155d3626a2..14f9f80e20a 100644
--- a/components/script/dom/windowproxy.rs
+++ b/components/script/dom/windowproxy.rs
@@ -4,7 +4,7 @@
 
 use crate::dom::bindings::cell::DomRefCell;
 use crate::dom::bindings::conversions::{root_from_handleobject, ToJSValConvertible};
-use crate::dom::bindings::error::{throw_dom_exception, Error};
+use crate::dom::bindings::error::{throw_dom_exception, Error, Fallible};
 use crate::dom::bindings::inheritance::Castable;
 use crate::dom::bindings::proxyhandler::fill_property_descriptor;
 use crate::dom::bindings::reflector::{DomObject, Reflector};
@@ -415,7 +415,7 @@ impl WindowProxy {
         url: USVString,
         target: DOMString,
         features: DOMString,
-    ) -> Option<DomRoot<WindowProxy>> {
+    ) -> Fallible<Option<DomRoot<WindowProxy>>> {
         // Step 4.
         let non_empty_target = match target.as_ref() {
             "" => DOMString::from("_blank"),
@@ -433,12 +433,12 @@ impl WindowProxy {
         // Step 10, 11
         let (chosen, new) = match self.choose_browsing_context(non_empty_target, noopener) {
             (Some(chosen), new) => (chosen, new),
-            (None, _) => return None,
+            (None, _) => return Ok(None),
         };
         // TODO Step 12, set up browsing context features.
         let target_document = match chosen.document() {
             Some(target_document) => target_document,
-            None => return None,
+            None => return Ok(None),
         };
         let target_window = target_document.window();
         // Step 13, and 14.4, will have happened elsewhere,
@@ -452,7 +452,7 @@ impl WindowProxy {
             // Step 14.1
             let url = match existing_document.url().join(&url) {
                 Ok(url) => url,
-                Err(_) => return None, // TODO: throw a  "SyntaxError" DOMException.
+                Err(_) => return Err(Error::Syntax),
             };
             // Step 14.3
             let referrer = if noreferrer {
@@ -479,10 +479,10 @@ impl WindowProxy {
         }
         if noopener {
             // Step 15 (Dis-owning has been done in create_auxiliary_browsing_context).
-            return None;
+            return Ok(None);
         }
         // Step 17.
-        return target_document.browsing_context();
+        return Ok(target_document.browsing_context());
     }
 
     // https://html.spec.whatwg.org/multipage/#the-rules-for-choosing-a-browsing-context-given-a-browsing-context-name