Skip to content

script: Ensure that leaving the WebView sets the cursor back to the default cursor#38759

Merged
mrobinson merged 1 commit intoservo:mainfrom
mrobinson:fix-move-leaving-webview
Aug 22, 2025
Merged

script: Ensure that leaving the WebView sets the cursor back to the default cursor#38759
mrobinson merged 1 commit intoservo:mainfrom
mrobinson:fix-move-leaving-webview

Conversation

@mrobinson
Copy link
Copy Markdown
Member

@mrobinson mrobinson commented Aug 18, 2025

This changes makes a variety of changes to ensure that the cursor is set
back to the default cursor when it leaves the WebView:

  1. Display list updates can come after a mouse leaves the WebView, so
    when refreshing the cursor after the update, base the updated cursor
    on the last hovered location in the DocumentEventHandler, rather
    than the compositor. This allows us to catch when the last hovered
    position is None (ie the cursor has left the WebView).
  2. When handling MouseLeftViewport events for the cursor leaving the
    entire WebView, properly set the
    MouseLeftViewport::focus_moving_to_another_iframe` on the input event
    passed to the script thread.
  3. When moving out of the WebView entirely, explicitly ask the
    embedder to set the cursor back to the default.

Testing: This change adds a unit test verifying this behavior.
Fixes: #38710.

@mrobinson mrobinson requested a review from gterzian as a code owner August 18, 2025 16:28
@mrobinson mrobinson force-pushed the fix-move-leaving-webview branch from 6780a6d to f7a1c67 Compare August 18, 2025 17:08
@mrobinson
Copy link
Copy Markdown
Member Author

I have added a unit test verifying the behavior of this change.

@mrobinson mrobinson force-pushed the fix-move-leaving-webview branch from f7a1c67 to 6838911 Compare August 21, 2025 10:37
Copy link
Copy Markdown
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job on the unit test!

test_evaluate_javascript_basic,
test_evaluate_javascript_panic,
test_theme_change,
test_theme_change,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test_theme_change,

/// Request that the given pipeline do a hit test at the location and reset the
/// cursor accordingly. This happens after a display list update is rendered.
RefreshCursor(PipelineId, Point2D<f32, CSSPixel>),
RefreshCursor(PipelineId),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure the docs for this message are up to date?

/// Ask that the given pipeline refreshes the cursor (after a display list render) based
/// on the hit test at the given point.
RefreshCursor(PipelineId, Point2D<f32, CSSPixel>),
RefreshCursor(PipelineId),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for these docs.

… default cursor

This changes makes a variety of changes to ensure that the cursor is set
back to the default cursor when it leaves the `WebView`:

1. Display list updates can come after a mouse leaves the `WebView`, so
   when refreshing the cursor after the update, base the updated cursor
   on the last hovered location in the `DocumentEventHandler`, rather
   than the compositor. This allows us to catch when the last hovered
   position is `None` (ie the cursor has left the `WebView`).
2. When handling `MouseLeftViewport` events for the cursor leaving the
   entire WebView, properly set the
   MouseLeftViewport::focus_moving_to_another_iframe` on the input event
   passed to the script thread.
3. When moving out of the `WebView` entirely, explicitly ask the
   embedder to set the cursor back to the default.

Signed-off-by: Martin Robinson <[email protected]>
@mrobinson mrobinson force-pushed the fix-move-leaving-webview branch from 6838911 to 6daf901 Compare August 22, 2025 07:16
@mrobinson mrobinson enabled auto-merge August 22, 2025 07:16
@mrobinson mrobinson added this pull request to the merge queue Aug 22, 2025
Merged via the queue into servo:main with commit 4784ff0 Aug 22, 2025
22 checks passed
@mrobinson mrobinson deleted the fix-move-leaving-webview branch August 22, 2025 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Element hover state and cursor style no longer cleared correctly on cursor left

2 participants