Use spec-aligned process for resolving history handling during navigation (#34747)

Makes use of the is_initial_about_blank property on Document in order to
determine whether the navigation should create a history entry, or replace
the current history entry.

Signed-off-by: Shane Handley <shanehandley@fastmail.com>

components/script/dom/document.rs

@@ -498,6 +498,8 @@ pub struct Document {
     visibility_state: Cell<DocumentVisibilityState>,
     /// <https://www.iana.org/assignments/http-status-codes/http-status-codes.xhtml>
     status_code: Option<u16>,
+    /// <https://html.spec.whatwg.org/multipage/#is-initial-about:blank>
+    is_initial_about_blank: Cell<bool>,
 }
 
 #[allow(non_snake_case)]
@@ -3219,6 +3221,7 @@ impl Document {
         referrer: Option<String>,
         status_code: Option<u16>,
         canceller: FetchCanceller,
+        is_initial_about_blank: bool,
     ) -> Document {
         let url = url.unwrap_or_else(|| ServoUrl::parse("about:blank").unwrap());
 
@@ -3246,6 +3249,7 @@ impl Document {
             .unwrap_or(UTF_8);
 
         let has_browsing_context = has_browsing_context == HasBrowsingContext::Yes;
+
         Document {
             node: Node::new_document_node(),
             document_or_shadow_root: DocumentOrShadowRoot::new(window),
@@ -3366,6 +3370,7 @@ impl Document {
             fonts: Default::default(),
             visibility_state: Cell::new(DocumentVisibilityState::Hidden),
             status_code,
+            is_initial_about_blank: Cell::new(is_initial_about_blank),
         }
     }
 
@@ -3480,6 +3485,7 @@ impl Document {
         referrer: Option<String>,
         status_code: Option<u16>,
         canceller: FetchCanceller,
+        is_initial_about_blank: bool,
         can_gc: CanGc,
     ) -> DomRoot<Document> {
         Self::new_with_proto(
@@ -3497,6 +3503,7 @@ impl Document {
             referrer,
             status_code,
             canceller,
+            is_initial_about_blank,
             can_gc,
         )
     }
@@ -3517,6 +3524,7 @@ impl Document {
         referrer: Option<String>,
         status_code: Option<u16>,
         canceller: FetchCanceller,
+        is_initial_about_blank: bool,
         can_gc: CanGc,
     ) -> DomRoot<Document> {
         let document = reflect_dom_object_with_proto(
@@ -3534,6 +3542,7 @@ impl Document {
                 referrer,
                 status_code,
                 canceller,
+                is_initial_about_blank,
             )),
             window,
             proto,
@@ -3657,6 +3666,7 @@ impl Document {
                     None,
                     None,
                     Default::default(),
+                    false,
                     can_gc,
                 );
                 new_doc
@@ -4178,6 +4188,11 @@ impl Document {
         self.upcast::<EventTarget>()
             .fire_bubbling_event(atom!("visibilitychange"), can_gc);
     }
+
+    /// <https://html.spec.whatwg.org/multipage/#is-initial-about:blank>
+    pub fn is_initial_about_blank(&self) -> bool {
+        self.is_initial_about_blank.get()
+    }
 }
 
 impl ProfilerMetadataFactory for Document {
@@ -4215,6 +4230,7 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
             None,
             None,
             Default::default(),
+            false,
             can_gc,
         ))
     }
@@ -5273,9 +5289,6 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
         let entry_responsible_document = GlobalScope::entry().as_window().Document();
 
         // Step 4
-        // This check is same-origin not same-origin-domain.
-        // https://github.com/whatwg/html/issues/2282
-        // https://github.com/whatwg/html/pull/2288
         if !self.origin.same_origin(&entry_responsible_document.origin) {
             return Err(Error::Security);
         }
@@ -5333,23 +5346,36 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
         // WPT selection/Document-open.html wants us to not clear it
         // as of Feb 1 2020
 
-        // Step 12
+        // Step 12. If document is fully active, then:
         if self.is_fully_active() {
+            // Step 12.1. Let newURL be a copy of entryDocument's URL.
             let mut new_url = entry_responsible_document.url();
+
+            // Step 12.2. If entryDocument is not document, then set newURL's fragment to null.
             if entry_responsible_document != DomRoot::from_ref(self) {
                 new_url.set_fragment(None);
             }
+
+            // Step 12.3. Run the URL and history update steps with document and newURL.
             // TODO: https://github.com/servo/servo/issues/21939
             self.set_url(new_url);
         }
 
-        // Step 13
+        // Step 13. Set document's is initial about:blank to false.
+        self.is_initial_about_blank.set(false);
+
+        // Step 14. If document's iframe load in progress flag is set, then set document's mute
+        // iframe load flag.
         // TODO: https://github.com/servo/servo/issues/21938
 
-        // Step 14
+        // Step 15: Set document to no-quirks mode.
         self.set_quirks_mode(QuirksMode::NoQuirks);
 
-        // Step 15
+        // Step 16. Create a new HTML parser and associate it with document. This is a
+        // script-created parser (meaning that it can be closed by the document.open() and
+        // document.close() methods, and that the tokenizer will wait for an explicit call to
+        // document.close() before emitting an end-of-file token). The encoding confidence is
+        // irrelevant.
         let resource_threads = self
             .window
             .upcast::<GlobalScope>()
@@ -5359,13 +5385,14 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
             DocumentLoader::new_with_threads(resource_threads, Some(self.url()));
         ServoParser::parse_html_script_input(self, self.url());
 
-        // Step 16
+        // Step 17. Set the insertion point to point at just before the end of the input stream
+        // (which at this point will be empty).
+        // Handled when creating the parser in step 16
+
+        // Step 18. Update the current document readiness of document to "loading".
         self.ready_state.set(DocumentReadyState::Loading);
 
-        // Step 17
-        // Handled when creating the parser in step 15
-
-        // Step 18
+        // Step 19. Return document.
         Ok(DomRoot::from_ref(self))
     }
 

components/script/dom/domimplementation.rs

@@ -164,6 +164,7 @@ impl DOMImplementationMethods<crate::DomTypeHolder> for DOMImplementation {
             None,
             None,
             Default::default(),
+            false,
             can_gc,
         );
 

components/script/dom/domparser.rs

@@ -87,6 +87,7 @@ impl DOMParserMethods<crate::DomTypeHolder> for DOMParser {
                     None,
                     None,
                     Default::default(),
+                    false,
                     can_gc,
                 );
                 ServoParser::parse_html_document(&document, Some(s), url, can_gc);
@@ -108,6 +109,7 @@ impl DOMParserMethods<crate::DomTypeHolder> for DOMParser {
                     None,
                     None,
                     Default::default(),
+                    false,
                     can_gc,
                 );
                 ServoParser::parse_xml_document(&document, Some(s), url, can_gc);

components/script/dom/node.rs

@@ -2360,6 +2360,7 @@ impl Node {
                     None,
                     document.status_code(),
                     Default::default(),
+                    false,
                     can_gc,
                 );
                 DomRoot::upcast::<Node>(document)

components/script/dom/servoparser/mod.rs

@@ -216,6 +216,7 @@ impl ServoParser {
             None,
             None,
             Default::default(),
+            false,
             can_gc,
         );
 

components/script/dom/window.rs

@@ -56,9 +56,9 @@ use script_layout_interface::{
 };
 use script_traits::webdriver_msg::{WebDriverJSError, WebDriverJSResult};
 use script_traits::{
-    ConstellationControlMsg, DocumentState, LoadData, NavigationHistoryBehavior, ScriptMsg,
-    ScriptToConstellationChan, ScrollState, StructuredSerializedData, Theme, WindowSizeData,
-    WindowSizeType,
+    ConstellationControlMsg, DocumentState, LoadData, LoadOrigin, NavigationHistoryBehavior,
+    ScriptMsg, ScriptToConstellationChan, ScrollState, StructuredSerializedData, Theme,
+    WindowSizeData, WindowSizeType,
 };
 use selectors::attr::CaseSensitivity;
 use servo_arc::Arc as ServoArc;
@@ -2336,13 +2336,18 @@ impl Window {
         can_gc: CanGc,
     ) {
         let doc = self.Document();
+
+        // Step 3. Let initiatorOriginSnapshot be sourceDocument's origin.
+        let initiator_origin_snapshot = &load_data.load_origin;
+
         // TODO: Important re security. See https://github.com/servo/servo/issues/23373
-        // Step 3: check that the source browsing-context is "allowed to navigate" this window.
+        // Step 5. check that the source browsing-context is "allowed to navigate" this window.
         if !force_reload &&
             load_data.url.as_url()[..Position::AfterQuery] ==
                 doc.url().as_url()[..Position::AfterQuery]
         {
             // Step 6
+            // TODO: Fragment handling appears to have moved to step 13
             if let Some(fragment) = load_data.url.fragment() {
                 self.send_to_constellation(ScriptMsg::NavigatedToFragment(
                     load_data.url.clone(),
@@ -2399,13 +2404,38 @@ impl Window {
                 // then put it in the delaying load events mode.
                 window_proxy.start_delaying_load_events_mode();
             }
-            // TODO: step 11, navigationType.
-            // Step 12, 13
+
+            // Step 11. If historyHandling is "auto", then:
+            let resolved_history_handling = if history_handling == NavigationHistoryBehavior::Auto {
+                // Step 11.1. If url equals navigable's active document's URL, and
+                // initiatorOriginSnapshot is same origin with targetNavigable's active document's
+                // origin, then set historyHandling to "replace".
+                // Note: `targetNavigable` is not actually defined in the spec, "active document" is
+                // assumed to be the correct reference based on WPT results
+                if let LoadOrigin::Script(initiator_origin) = initiator_origin_snapshot {
+                    if load_data.url == doc.url() && initiator_origin.same_origin(doc.origin()) {
+                        NavigationHistoryBehavior::Replace
+                    } else {
+                        NavigationHistoryBehavior::Push
+                    }
+                } else {
+                    // Step 11.2. Otherwise, set historyHandling to "push".
+                    NavigationHistoryBehavior::Push
+                }
+            // Step 12. If the navigation must be a replace given url and navigable's active
+            // document, then set historyHandling to "replace".
+            } else if load_data.url.scheme() == "javascript" || doc.is_initial_about_blank() {
+                NavigationHistoryBehavior::Replace
+            } else {
+                NavigationHistoryBehavior::Push
+            };
+
+            // Step 13
             ScriptThread::navigate(
                 window_proxy.browsing_context_id(),
                 pipeline_id,
                 load_data,
-                history_handling,
+                resolved_history_handling,
             );
         };
     }

components/script/dom/xmldocument.rs

@@ -57,6 +57,7 @@ impl XMLDocument {
                 None,
                 None,
                 Default::default(),
+                false,
             ),
         }
     }

components/script/dom/xmlhttprequest.rs

@@ -1534,6 +1534,7 @@ impl XMLHttpRequest {
             None,
             None,
             Default::default(),
+            false,
             can_gc,
         )
     }

components/script/script_thread.rs

@@ -3134,6 +3134,8 @@ impl ScriptThread {
             .as_ref()
             .map(|referrer| referrer.clone().into_string());
 
+        let is_initial_about_blank = final_url.as_str() == "about:blank";
+
         let document = Document::new(
             &window,
             HasBrowsingContext::Yes,
@@ -3148,6 +3150,7 @@ impl ScriptThread {
             referrer,
             Some(metadata.status.raw_code()),
             incomplete.canceller,
+            is_initial_about_blank,
             can_gc,
         );
 

tests/wpt/meta/html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/iframe-src-204.html.ini

@@ -1,9 +1,3 @@
 [iframe-src-204.html]
   [Navigating to a different document with window.open]
     expected: FAIL
-
-  [Navigating to a different document with link click]
-    expected: FAIL
-
-  [Navigating to a different document with form submission]
-    expected: FAIL

tests/wpt/meta/html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/iframe-src-aboutblank-wait-for-load.html.ini

@@ -2,17 +2,5 @@
   [Navigating to a different document with src]
     expected: FAIL
 
-  [Navigating to a different document with location.href]
-    expected: FAIL
-
-  [Navigating to a different document with location.assign]
-    expected: FAIL
-
   [Navigating to a different document with window.open]
     expected: FAIL
-
-  [Navigating to a different document with link click]
-    expected: FAIL
-
-  [Navigating to a different document with form submission]
-    expected: FAIL

tests/wpt/meta/html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/window-open-aboutblank.html.ini

@@ -1,6 +0,0 @@
-[window-open-aboutblank.html]
-  [location.href]
-    expected: FAIL
-
-  [location.assign]
-    expected: FAIL

tests/wpt/meta/html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/window-open-nourl.html.ini

@@ -1,6 +0,0 @@
-[window-open-nourl.html]
-  [location.href]
-    expected: FAIL
-
-  [location.assign]
-    expected: FAIL