Skip to content

Conversation

@simonwuelker
Copy link
Contributor

Previously servo would allow whitespace in between components of an xpath expression, but not around it.

Testing: New web platform tests start to pass
Part of #34527

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Sep 20, 2025
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Sep 20, 2025
Copy link
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.

This change itself doesn't allow whitespace at end of path expression.

Or it already did elsewhere?

@simonwuelker
Copy link
Contributor Author

This change itself doesn't allow whitespace at end of path expression.

Or it already did elsewhere?

ws accepts whitespace both before and after the supplied parser function:

fn ws<'a, F, O, E>(inner: F) -> impl Parser<&'a str, Output = O, Error = E>
where
E: NomParseError<&'a str>,
F: Parser<&'a str, Output = O, Error = E>,
{
delimited(multispace0, inner, multispace0)
}

(I'm not super happy with the structure of the parser, but thats a bigger issue...)

Accepting whitespace after the expression is also verified by the web platform tests:

parse(' \'a"bc\' ');
parse(' "a\'bc" ');

@simonwuelker simonwuelker added this pull request to the merge queue Sep 20, 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 Sep 20, 2025
Merged via the queue into servo:main with commit bbceb0f Sep 20, 2025
28 checks passed
@simonwuelker simonwuelker deleted the xpath-whitespace branch September 20, 2025 23:39
@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 Sep 20, 2025
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.

3 participants