From f155c95e1b4df92f9031e19f07acc0a50492438b Mon Sep 17 00:00:00 2001
From: batu_hoang <55729155+longvatrong111@users.noreply.github.com>
Date: Tue, 15 Jul 2025 14:51:05 +0800
Subject: [PATCH] webdriver: Element click waits for navigation complete
 (#37935)

Step 11 in https://w3c.github.io/webdriver/#dfn-element-click

> [Try](https://w3c.github.io/webdriver/#dfn-try) to [wait for
navigation to
complete](https://w3c.github.io/webdriver/#dfn-wait-for-navigation-to-complete)
with session.

This fixes issue in which element_click triggers navigation, but
incoming commands still interact with old page.

Testing:
https://github.com/longvatrong111/servo/actions/runs/16175767947
https://github.com/longvatrong111/servo/actions/runs/16175770044

Signed-off-by: batu_hoang <longvatrong111@gmail.com>
---
 components/script/script_thread.rs      |   7 ++
 components/script/webdriver_handlers.rs |  21 +++-
 components/shared/embedder/webdriver.rs |   3 +
 components/webdriver_server/lib.rs      | 156 ++++++++++++++----------
 ports/servoshell/desktop/app.rs         |   6 +
 ports/servoshell/desktop/app_state.rs   |   7 ++
 6 files changed, 137 insertions(+), 63 deletions(-)

diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs
index 809ded115de..18bf170f1f3 100644
--- a/components/script/script_thread.rs
+++ b/components/script/script_thread.rs
@@ -2499,6 +2499,13 @@ impl ScriptThread {
                 reply,
                 can_gc,
             ),
+            WebDriverScriptCommand::IsDocumentReadyStateComplete(response_sender) => {
+                webdriver_handlers::handle_try_wait_for_document_navigation(
+                    &documents,
+                    pipeline_id,
+                    response_sender,
+                )
+            },
             _ => (),
         }
     }
diff --git a/components/script/webdriver_handlers.rs b/components/script/webdriver_handlers.rs
index 7c6677cc6b9..058b4d2245e 100644
--- a/components/script/webdriver_handlers.rs
+++ b/components/script/webdriver_handlers.rs
@@ -33,7 +33,9 @@ use webdriver::error::ErrorStatus;
 use crate::document_collection::DocumentCollection;
 use crate::dom::bindings::codegen::Bindings::CSSStyleDeclarationBinding::CSSStyleDeclarationMethods;
 use crate::dom::bindings::codegen::Bindings::DOMRectBinding::DOMRectMethods;
-use crate::dom::bindings::codegen::Bindings::DocumentBinding::DocumentMethods;
+use crate::dom::bindings::codegen::Bindings::DocumentBinding::{
+    DocumentMethods, DocumentReadyState,
+};
 use crate::dom::bindings::codegen::Bindings::ElementBinding::ElementMethods;
 use crate::dom::bindings::codegen::Bindings::HTMLElementBinding::HTMLElementMethods;
 use crate::dom::bindings::codegen::Bindings::HTMLInputElementBinding::HTMLInputElementMethods;
@@ -1717,3 +1719,20 @@ pub(crate) fn handle_is_selected(
         )
         .unwrap();
 }
+
+pub(crate) fn handle_try_wait_for_document_navigation(
+    documents: &DocumentCollection,
+    pipeline: PipelineId,
+    reply: IpcSender<bool>,
+) {
+    let document = match documents.find_document(pipeline) {
+        Some(document) => document,
+        None => {
+            return reply.send(false).unwrap();
+        },
+    };
+
+    let wait_for_document_ready = document.ReadyState() != DocumentReadyState::Complete;
+
+    reply.send(wait_for_document_ready).unwrap();
+}
diff --git a/components/shared/embedder/webdriver.rs b/components/shared/embedder/webdriver.rs
index 9ffd187706f..897289da901 100644
--- a/components/shared/embedder/webdriver.rs
+++ b/components/shared/embedder/webdriver.rs
@@ -127,6 +127,8 @@ pub enum WebDriverCommandMsg {
         IpcSender<Result<(), ()>>,
     ),
     GetAlertText(WebViewId, IpcSender<Result<String, ()>>),
+    AddLoadStatusSender(WebViewId, IpcSender<WebDriverLoadStatus>),
+    RemoveLoadStatusSender(WebViewId),
 }
 
 #[derive(Debug, Deserialize, Serialize)]
@@ -202,6 +204,7 @@ pub enum WebDriverScriptCommand {
     GetTitle(IpcSender<String>),
     /// Match the element type before sending the event for webdriver `element send keys`.
     WillSendKeys(String, String, bool, IpcSender<Result<bool, ErrorStatus>>),
+    IsDocumentReadyStateComplete(IpcSender<bool>),
 }
 
 #[derive(Clone, Debug, Deserialize, Serialize)]
diff --git a/components/webdriver_server/lib.rs b/components/webdriver_server/lib.rs
index 96075e9f222..e93d19a0101 100644
--- a/components/webdriver_server/lib.rs
+++ b/components/webdriver_server/lib.rs
@@ -1871,9 +1871,11 @@ impl Handler {
         Ok(WebDriverResponse::Void)
     }
 
-    // https://w3c.github.io/webdriver/#element-click
+    /// <https://w3c.github.io/webdriver/#element-click>
     fn handle_element_click(&mut self, element: &WebElement) -> WebDriverResult<WebDriverResponse> {
         let (sender, receiver) = ipc::channel().unwrap();
+        let webview_id = self.session()?.webview_id;
+        let browsing_context_id = self.session()?.browsing_context_id;
 
         // Steps 1 - 7 + Step 8 for <option>
         let cmd = WebDriverScriptCommand::ElementClick(element.to_string(), sender);
@@ -1882,72 +1884,36 @@ impl Handler {
         match wait_for_script_response(receiver)? {
             Ok(element_id) => match element_id {
                 Some(element_id) => {
-                    let id = Uuid::new_v4().to_string();
-                    // Step 8 for elements other than <option>
-                    // Step 8.1
-                    self.session_mut()?.input_state_table.borrow_mut().insert(
-                        id.clone(),
-                        InputSourceState::Pointer(PointerInputState::new(PointerType::Mouse)),
-                    );
+                    // Load status sender should be set up before we dispatch actions
+                    self.send_message_to_embedder(WebDriverCommandMsg::AddLoadStatusSender(
+                        webview_id,
+                        self.load_status_sender.clone(),
+                    ))?;
 
-                    // Step 8.7. Construct a pointer move action.
-                    // Step 8.8. Set a property x to 0 on pointer move action.
-                    // Step 8.9. Set a property y to 0 on pointer move action.
-                    // Step 8.10. Set a property origin to element on pointer move action.
-                    let pointer_move_action = PointerMoveAction {
-                        duration: None,
-                        origin: PointerOrigin::Element(WebElement(element_id)),
-                        x: 0.0,
-                        y: 0.0,
-                        ..Default::default()
-                    };
+                    self.perform_element_click(element_id)?;
 
-                    // Step 8.11. Construct pointer down action.
-                    // Step 8.12. Set a property button to 0 on pointer down action.
-                    let pointer_down_action = PointerDownAction {
-                        button: i16::from(MouseButton::Left) as u64,
-                        ..Default::default()
-                    };
+                    // Step 11. Try to wait for navigation to complete with session.
+                    // The most reliable way to try to wait for a potential navigation
+                    // which is caused by element click to check with script thread
+                    let (sender, receiver) = ipc::channel().unwrap();
+                    self.send_message_to_embedder(WebDriverCommandMsg::ScriptCommand(
+                        browsing_context_id,
+                        WebDriverScriptCommand::IsDocumentReadyStateComplete(sender),
+                    ))?;
 
-                    // Step 8.13. Construct pointer up action.
-                    // Step 8.14. Set a property button to 0 on pointer up action.
-                    let pointer_up_action = PointerUpAction {
-                        button: i16::from(MouseButton::Left) as u64,
-                        ..Default::default()
-                    };
-
-                    let action_sequence = ActionSequence {
-                        id: id.clone(),
-                        actions: ActionsType::Pointer {
-                            parameters: PointerActionParameters {
-                                pointer_type: PointerType::Mouse,
-                            },
-                            actions: vec![
-                                PointerActionItem::Pointer(PointerAction::Move(
-                                    pointer_move_action,
-                                )),
-                                PointerActionItem::Pointer(PointerAction::Down(
-                                    pointer_down_action,
-                                )),
-                                PointerActionItem::Pointer(PointerAction::Up(pointer_up_action)),
-                            ],
-                        },
-                    };
-
-                    // Step 8.16. Dispatch a list of actions with session's current browsing context
-                    let actions_by_tick = self.actions_by_tick_from_sequence(vec![action_sequence]);
-                    if let Err(e) =
-                        self.dispatch_actions(actions_by_tick, self.session()?.browsing_context_id)
-                    {
-                        log::error!("handle_element_click: dispatch_actions failed: {:?}", e);
+                    if wait_for_script_response(receiver)? {
+                        self.load_status_receiver.recv().map_err(|_| {
+                            WebDriverError::new(
+                                ErrorStatus::UnknownError,
+                                "Failed to receive load status",
+                            )
+                        })?;
+                    } else {
+                        self.send_message_to_embedder(
+                            WebDriverCommandMsg::RemoveLoadStatusSender(webview_id),
+                        )?;
                     }
 
-                    // Step 8.17 Remove an input source with input state and input id.
-                    self.session_mut()?
-                        .input_state_table
-                        .borrow_mut()
-                        .remove(&id);
-
                     // Step 13
                     Ok(WebDriverResponse::Void)
                 },
@@ -1958,6 +1924,72 @@ impl Handler {
         }
     }
 
+    fn perform_element_click(&mut self, element: String) -> WebDriverResult<WebDriverResponse> {
+        // Step 8 for elements other than <option>
+        let id = Uuid::new_v4().to_string();
+
+        // Step 8.1
+        self.session_mut()?.input_state_table.borrow_mut().insert(
+            id.clone(),
+            InputSourceState::Pointer(PointerInputState::new(PointerType::Mouse)),
+        );
+
+        // Step 8.7. Construct a pointer move action.
+        // Step 8.8. Set a property x to 0 on pointer move action.
+        // Step 8.9. Set a property y to 0 on pointer move action.
+        // Step 8.10. Set a property origin to element on pointer move action.
+        let pointer_move_action = PointerMoveAction {
+            duration: None,
+            origin: PointerOrigin::Element(WebElement(element)),
+            x: 0.0,
+            y: 0.0,
+            ..Default::default()
+        };
+
+        // Step 8.11. Construct pointer down action.
+        // Step 8.12. Set a property button to 0 on pointer down action.
+        let pointer_down_action = PointerDownAction {
+            button: i16::from(MouseButton::Left) as u64,
+            ..Default::default()
+        };
+
+        // Step 8.13. Construct pointer up action.
+        // Step 8.14. Set a property button to 0 on pointer up action.
+        let pointer_up_action = PointerUpAction {
+            button: i16::from(MouseButton::Left) as u64,
+            ..Default::default()
+        };
+
+        let action_sequence = ActionSequence {
+            id: id.clone(),
+            actions: ActionsType::Pointer {
+                parameters: PointerActionParameters {
+                    pointer_type: PointerType::Mouse,
+                },
+                actions: vec![
+                    PointerActionItem::Pointer(PointerAction::Move(pointer_move_action)),
+                    PointerActionItem::Pointer(PointerAction::Down(pointer_down_action)),
+                    PointerActionItem::Pointer(PointerAction::Up(pointer_up_action)),
+                ],
+            },
+        };
+
+        // Step 8.16. Dispatch a list of actions with session's current browsing context
+        let actions_by_tick = self.actions_by_tick_from_sequence(vec![action_sequence]);
+        if let Err(e) = self.dispatch_actions(actions_by_tick, self.session()?.browsing_context_id)
+        {
+            log::error!("handle_element_click: dispatch_actions failed: {:?}", e);
+        }
+
+        // Step 8.17 Remove an input source with input state and input id.
+        self.session_mut()?
+            .input_state_table
+            .borrow_mut()
+            .remove(&id);
+
+        Ok(WebDriverResponse::Void)
+    }
+
     fn take_screenshot(&self, rect: Option<Rect<f32, CSSPixel>>) -> WebDriverResult<String> {
         // Step 1. If session's current top-level browsing context is no longer open,
         // return error with error code no such window.
diff --git a/ports/servoshell/desktop/app.rs b/ports/servoshell/desktop/app.rs
index e4cb310fc26..e264e62ff0b 100644
--- a/ports/servoshell/desktop/app.rs
+++ b/ports/servoshell/desktop/app.rs
@@ -454,6 +454,12 @@ impl App {
                         warn!("Failed to send response of GetFocusedWebView: {error}");
                     };
                 },
+                WebDriverCommandMsg::AddLoadStatusSender(webview_id, load_status_sender) => {
+                    running_state.set_load_status_sender(webview_id, load_status_sender);
+                },
+                WebDriverCommandMsg::RemoveLoadStatusSender(webview_id) => {
+                    running_state.remove_load_status_sender(webview_id);
+                },
                 WebDriverCommandMsg::LoadUrl(webview_id, url, load_status_sender) => {
                     if let Some(webview) = running_state.webview_by_id(webview_id) {
                         webview.load(url.into_url());
diff --git a/ports/servoshell/desktop/app_state.rs b/ports/servoshell/desktop/app_state.rs
index e9d2da88805..b8b439db23a 100644
--- a/ports/servoshell/desktop/app_state.rs
+++ b/ports/servoshell/desktop/app_state.rs
@@ -470,6 +470,13 @@ impl RunningAppState {
             });
         }
     }
+
+    pub(crate) fn remove_load_status_sender(&self, webview_id: WebViewId) {
+        self.webdriver_senders
+            .borrow_mut()
+            .load_status_senders
+            .remove(&webview_id);
+    }
 }
 
 struct ServoShellServoDelegate;