Skip to content

script: Enable xpath by default#40212

Merged
simonwuelker merged 1 commit intoservo:mainfrom
simonwuelker:xpath-enable-by-default
Oct 27, 2025
Merged

script: Enable xpath by default#40212
simonwuelker merged 1 commit intoservo:mainfrom
simonwuelker:xpath-enable-by-default

Conversation

@simonwuelker
Copy link
Copy Markdown
Contributor

@simonwuelker simonwuelker commented Oct 27, 2025

Fixes: #34527

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 27, 2025
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Oct 27, 2025
yezhizhen
yezhizhen approved these changes Oct 27, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@yezhizhen yezhizhen left a comment

Choose a reason for hiding this comment

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

We should also remove it from EXPERIMENTAL_PREFS in servoshell/prefs.rs.

jschwe
jschwe reviewed Oct 27, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@jschwe jschwe left a comment

Choose a reason for hiding this comment

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

What about keeping the pref for now, and only enabling it by default? This would allow us to turn it off again if it is causing problems in some scenario.
We could then completely remove the pref after some grace period.

@simonwuelker
Copy link
Copy Markdown
Contributor Author

simonwuelker commented Oct 27, 2025

What about keeping the pref for now, and only enabling it by default? This would allow us to turn it off again if it is causing problems in some scenario. We could then completely remove the pref after some grace period.

We did this for shadow dom, but I can't imagine a scenario where where xpath would cause major issues... We can always revert this commit ig?

@simonwuelker
Enable xpath by default
4b6bb93 
Signed-off-by: Simon Wülker <[email protected]>
@simonwuelker simonwuelker force-pushed the xpath-enable-by-default branch from c963725 to 4b6bb93 Compare October 27, 2025 18:23
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 27, 2025
@simonwuelker simonwuelker added this pull request to the merge queue Oct 27, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 27, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 27, 2025
@servo-highfive servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Oct 27, 2025
@simonwuelker simonwuelker added this pull request to the merge queue Oct 27, 2025
@servo-highfive servo-highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Oct 27, 2025
Merged via the queue into servo:main with commit f9f2a13 Oct 27, 2025
39 checks passed
@simonwuelker simonwuelker deleted the xpath-enable-by-default branch October 27, 2025 20:07
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 27, 2025
@yezhizhen
Copy link
Copy Markdown
Member

yezhizhen commented Oct 28, 2025

What about keeping the pref for now, and only enabling it by default? This would allow us to turn it off again if it is causing problems in some scenario. We could then completely remove the pref after some grace period.

XPath should be fine. It is mostly used for automation/testing but not website itself, so should be covered by existing WPT/webdriver test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jschwe jschwe jschwe left review comments

@yezhizhen yezhizhen yezhizhen approved these changes

@Taym95 Taym95 Taym95 approved these changes

Assignees

No one assigned

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Enable XPath by default

5 participants

@simonwuelker @yezhizhen @Taym95 @jschwe @servo-highfive