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

Pseudo-element selector should not be accepted when there are spaces between colon and pseudo-element name #15335

Closed
upsuper opened this issue Feb 2, 2017 · 6 comments
Labels
A-content/css Interacting with CSS from web content (parsing, serializing, introspection) A-content/parsers Related to parsing HTML and XML C-assigned There is someone working on resolving the issue E-less-complex Straightforward. Recommended for a new contributor.

Comments

@upsuper
Copy link
Contributor

upsuper commented Feb 2, 2017

Servo fails with the following test:

<!DOCTYPE html>
<style>
div::before {
  color: green;
  content: "PASS";
}
div:: before {
  color: red;
  content: "FAIL";
}
</style>
<div></div>

It is probably an issue in the cssparser. And this could probably be an easy bug.

cc @SimonSapin

@upsuper upsuper added the A-content/css Interacting with CSS from web content (parsing, serializing, introspection) label Feb 2, 2017
@SimonSapin SimonSapin added A-content/parsers Related to parsing HTML and XML E-less-complex Straightforward. Recommended for a new contributor. labels Feb 2, 2017
@highfive
Copy link

highfive commented Feb 2, 2017

Please make a comment here if you intend to work on this issue. Thank you!

@SimonSapin
Copy link
Member

SimonSapin commented Feb 2, 2017

In https://github.com/servo/rust-selectors, in src/parser.rs, in the test_parsing function, a new test should be added near the existing parse("::before") test to check that parse(":: before") returns Err(()) as it should. Add the URL of this issue in a comment.

Once that test has been observed failing, the bug needs to be fixed. It’s in the parse_one_simple_selector function of the same file. In the case where two consecutive Token::Colon are found, the code currently calls input.next() (and then parse_pseudo_element if it finds an identifier). That calls need to be changed to input.next_including_whitespace(). That change should make the new test pass.

@atheed
Copy link
Contributor

atheed commented Feb 2, 2017

I'd like to take this.

@SimonSapin SimonSapin added the C-assigned There is someone working on resolving the issue label Feb 2, 2017
@SimonSapin
Copy link
Member

Go ahead!

@SimonSapin
Copy link
Member

servo/rust-selectors#103 is merged and v0.15.2 on crates.io. When updating Servo we should probably include the test case above.

@upsuper
Copy link
Contributor Author

upsuper commented Feb 8, 2017

It seems this has been fixed. Closing.

@upsuper upsuper closed this as completed Feb 8, 2017
bors-servo pushed a commit that referenced this issue Sep 17, 2017
Land the test cases that should've landed with the selectors update

PR for #15448
- Added test for pseudo-element with whitespace #15335
- Added tests for empty values in `[att^=""]`, `[att$=""]` and `[att*=""]` selectors #15394 (tests for `[att~=""]` have been added with #15440)

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #15448
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18542)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-content/css Interacting with CSS from web content (parsing, serializing, introspection) A-content/parsers Related to parsing HTML and XML C-assigned There is someone working on resolving the issue E-less-complex Straightforward. Recommended for a new contributor.
Projects
None yet
Development

No branches or pull requests

4 participants