Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve support for actions involving multiple navigables #1875

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jgraham
Copy link
Member

@jgraham jgraham commented Jan 7, 2025

This makes the following changes:

  • Actions are always dispatched first to the top-level traversable rather than directly to the target navigable. It is assumed that the action will end up interacting with the target traversable, but may not e.g. if there is an element obscuring the target, or the focus is in the wrong place.

  • To support the above, coordinates are computed relative to the viewport of the top-level traversable rather than the target navigable.

  • A new coordinate origin type is introduced which has the form: { type: "viewport", context: <context id> }

    This is designed to allow targeting inside a specific iframe without that iframe needing to be the target of the overall actions chain.

The existing behaviour that if the main target navigable is deleted the action chain is terminated remains. This makes the behaviour consistent between top-level traversables and iframes, and also between different actions.


Preview | Diff

This makes the following changes:

* Actions are always dispatched first to the top-level traversable
rather than directly to the target navigable. It is assumed that the
action will end up interacting with the target traversable, but may
not e.g. if there is an element obscuring the target, or the focus is
in the wrong place.

* To support the above, coordinates are computed relative to the
viewport of the top-level traversable rather than the target
navigable.

* A new coordinate origin type is introduced which has the form:
  ```
  {
    type: "viewport",
    context: <context id>
  }
  ```

  This is designed to allow targeting inside a specific iframe without
  that iframe needing to be the target of the overall actions chain.

The existing behaviour that if the main target navigable is deleted
the action chain is terminated remains. This makes the behaviour
consistent between top-level traversables and iframes, and also
between different actions.
@jgraham
Copy link
Member Author

jgraham commented Jan 7, 2025

There are still a number of open questions here:

  • The new coordinate origin is the first thing to introduce the idea of a global id for navigables (particularly iframes) into WebDriver classic. The design is a natural fit to BiDi, but there's a question about whether it's appropriate for Classic.
  • Similarly the current element origin depends on the element being in the target navigable. In BiDi it's rather easy to add a context parameter to the element origin to specify where to look for the element, but that doesn't work for web elements. The alternative would be to allow searching the entire tree of navigables under the top-level traversable for the element. That would be easy to use, but might have perf implications.
  • It's unclear that we're maintaining the current behaviour for elements that are inside the viewport of an iframe but outside the top-level viewport (e.g. consider a case where a button is inside an iframe, and would be visible if you scrolled the top level document, but where the entire iframe is currently entirely out of view).
  • I worry a bit about how backwards incompatible the change is. I know it's similar to what Chrome is already doing, but I want to check that there aren't any features of the Chrome implementation that aren't captured here (e.g. implicitly moving focus to the target navigable at the start of the action chain). With wheel and pointer events the fact that they might be captured by the top-level document is considered a feature, but I'm not so sure about key events.

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.

1 participant