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

New characters escaped in serialize_name #104

Merged
merged 1 commit into from May 6, 2016

Conversation

@mskrzypkows
Copy link
Contributor

mskrzypkows commented May 4, 2016

Added escape of NULL (U+0000) char and escape for characters
in the range [\1-\1f](U+0001 to U+001F) or U+007F
according to https://drafts.csswg.org/cssom/#serialize-an-identifier
Needed for proper CSS tests servo/servo#10685


This change is Reviewable

@nox
Copy link
Member

nox commented May 4, 2016

The code looks correct, but could you add some unit tests please?

@nox nox self-assigned this May 4, 2016
@mskrzypkows
Copy link
Contributor Author

mskrzypkows commented May 4, 2016

OK I'll add, forgot about it.

@SimonSapin
Copy link
Member

SimonSapin commented May 4, 2016

Please add unit tests in src/tests.rs of the form:

#[test]
fn identifier_serialization() {
    assert_eq!(Token::Ident("\0").to_css_string(), "\u{FFFD}");
    assert_eq!(Token::Ident("a\0").to_css_string(), "a\u{FFFD}");
    // ...
}

with one assert_eq! for each assert_equals in https://github.com/servo/servo/blob/master/tests/wpt/css-tests/cssom-1_dev/html/escape.htm, except for "Number of arguments", "String conversion", and "Surrogates" ones. (But add one with "\u{1D306}" instead of surrogates.)

Maciej Skrzypkowski
Added escape of NULL (U+0000) char and escape for characters
in the range [\1-\1f] (U+0001 to U+001F) or U+007F
according to https://drafts.csswg.org/cssom/#serialize-an-identifier
Needed for proper CSS tests servo/servo#10685
@mskrzypkows mskrzypkows force-pushed the mskrzypkows:serialize_name branch from 2532291 to 2f48ad9 May 6, 2016
@mskrzypkows
Copy link
Contributor Author

mskrzypkows commented May 6, 2016

I've added tests.

@SimonSapin
Copy link
Member

SimonSapin commented May 6, 2016

Great, thanks!

@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented May 6, 2016

📌 Commit 2f48ad9 has been approved by SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented May 6, 2016

Testing commit 2f48ad9 with merge 713c762...

bors-servo added a commit that referenced this pull request May 6, 2016
New characters escaped in serialize_name

Added escape of NULL (U+0000) char and escape for characters
in the range [\1-\1f] (U+0001 to U+001F) or U+007F
according to https://drafts.csswg.org/cssom/#serialize-an-identifier
Needed for proper CSS tests servo/servo#10685

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-cssparser/104)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 6, 2016

☀️ Test successful - travis

@bors-servo bors-servo merged commit 2f48ad9 into servo:master May 6, 2016
1 of 2 checks passed
1 of 2 checks passed
homu Testing commit 2f48ad9 with merge 713c762...
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mskrzypkows mskrzypkows deleted the mskrzypkows:serialize_name branch May 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.