Skip to content

wdspec tests: Null "implicit wait" should not have timer#57332

Closed
servo-wpt-sync wants to merge 1 commit intoweb-platform-tests:masterfrom
servo:servo_export_42158
Closed

wdspec tests: Null "implicit wait" should not have timer#57332
servo-wpt-sync wants to merge 1 commit intoweb-platform-tests:masterfrom
servo:servo_export_42158

Conversation

@servo-wpt-sync
Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

The review process for this patch is being conducted in the Servo project.

@yezhizhen
Copy link
Copy Markdown
Contributor

cc @whimboo @jgraham

@whimboo
Copy link
Copy Markdown
Contributor

whimboo commented Jan 27, 2026

When implicit wait is None, we should not wait at all: #57151 (comment)

See step 5 of https://w3c.github.io/webdriver/#dfn-find Reviewed in servo/servo#42158

I think you misinterpret these referenced lines. We only start the timer when the value is not None. That means without a timer we should try to find the element infinitely:

If timeout is not null:

[Start the timer](https://w3c.github.io/webdriver/#dfn-start-the-timer) with timer and timeout.

@yezhizhen
Copy link
Copy Markdown
Contributor

yezhizhen commented Jan 27, 2026

That means without a timer we should try to find the element infinitely:

Ah, that makes sense. I wonder if we should make spec clearer with null timeout.

@servo-wpt-sync servo-wpt-sync deleted the servo_export_42158 branch January 27, 2026 10:24
github-merge-queue bot pushed a commit to servo/servo that referenced this pull request Jan 30, 2026
So far for `None`
[timeout](https://w3c.github.io/webdriver/#dfn-timeouts-configuration),
we've treated it effectively as no wait which is wrong:
web-platform-tests/wpt#57332 (comment).

This PR makes all `None` timeouts wait indefinitely. The funny part is,
we somehow have only done it correctly for `pageload`:
https://w3c.github.io/webdriver/#dfn-timeouts-configuration

Testing: We add test for script execution with `None` script timeout.

---------

Signed-off-by: Euclid Ye <[email protected]>
RichardTjokroutomo pushed a commit to RichardTjokroutomo/servo that referenced this pull request Feb 6, 2026
So far for `None`
[timeout](https://w3c.github.io/webdriver/#dfn-timeouts-configuration),
we've treated it effectively as no wait which is wrong:
web-platform-tests/wpt#57332 (comment).

This PR makes all `None` timeouts wait indefinitely. The funny part is,
we somehow have only done it correctly for `pageload`:
https://w3c.github.io/webdriver/#dfn-timeouts-configuration

Testing: We add test for script execution with `None` script timeout.

---------

Signed-off-by: Euclid Ye <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants