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

Update cssparser to 0.11 #15735

Merged
merged 5 commits into from Feb 26, 2017

Conversation

@SimonSapin
Copy link
Member

commented Feb 25, 2017

Depends on servo/rust-cssparser#122.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #__ (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because _____

This change is Reviewable

@highfive

This comment has been minimized.

Copy link

commented Feb 25, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/values/specified/basic_shape.rs, components/style/properties/longhand/text.mako.rs, components/style/values/specified/length.rs, components/style/properties/properties.mako.rs, components/style/gecko/media_queries.rs, components/style/Cargo.toml, components/style/lib.rs, components/style/values/specified/image.rs, components/style/properties/longhand/font.mako.rs, components/style/servo/media_queries.rs, components/style/properties/longhand/effects.mako.rs, components/style/values/specified/mod.rs, components/style/properties/longhand/counters.mako.rs, components/style/properties/longhand/box.mako.rs, components/style/supports.rs, components/style/values/mod.rs, components/style/values/specified/align.rs, components/style/properties/helpers/animated_properties.mako.rs, components/style/values/specified/position.rs, components/style/properties/longhand/inherited_svg.mako.rs
  • @KiChjang: components/script/dom/csskeyframesrule.rs, components/script/lib.rs, components/script/Cargo.toml
  • @fitzgen: components/script/dom/csskeyframesrule.rs, components/script/lib.rs, components/script/Cargo.toml
  • @emilio: components/style/values/specified/basic_shape.rs, components/style/properties/longhand/text.mako.rs, components/style/values/specified/length.rs, components/style/properties/properties.mako.rs, components/style/gecko/media_queries.rs, components/style/Cargo.toml, components/style/lib.rs, components/style/values/specified/image.rs, components/style/properties/longhand/font.mako.rs, components/style/servo/media_queries.rs, components/style/properties/longhand/effects.mako.rs, components/style/values/specified/mod.rs, components/style/properties/longhand/counters.mako.rs, components/style/properties/longhand/box.mako.rs, components/style/supports.rs, components/style/values/mod.rs, components/style/values/specified/align.rs, components/style/properties/helpers/animated_properties.mako.rs, components/style/values/specified/position.rs, components/style/properties/longhand/inherited_svg.mako.rs
@highfive

This comment has been minimized.

Copy link

commented Feb 25, 2017

warning Warning warning

  • These commits modify style and script code, but no tests are modified. Please consider adding a test!
@SimonSapin

This comment has been minimized.

Copy link
Member Author

commented Feb 25, 2017

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Feb 25, 2017

⌛️ Trying commit 84aa188 with merge 1c993ff...

bors-servo added a commit that referenced this pull request Feb 25, 2017
Update cssparser to 0.11

<!-- Please describe your changes on the following line: -->

Depends on servo/rust-cssparser#122.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@SimonSapin SimonSapin force-pushed the cssparserup branch from afd5838 to eacc12d Feb 25, 2017
@SimonSapin

This comment has been minimized.

Copy link
Member Author

commented Feb 25, 2017

This is ready to land. r? @Manishearth

@highfive highfive assigned Manishearth and unassigned KiChjang Feb 25, 2017
@emilio

This comment has been minimized.

Copy link
Member

commented Feb 25, 2017

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Feb 25, 2017

📌 Commit eacc12d has been approved by emilio

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Feb 25, 2017

⌛️ Testing commit eacc12d with merge 11f362a...

bors-servo added a commit that referenced this pull request Feb 25, 2017
Update cssparser to 0.11

<!-- Please describe your changes on the following line: -->

<s>Depends on https://github.com/servo/rust-cssparser/pull/122.</s>

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/15735)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Feb 25, 2017

💔 Test failed - linux-rel-wpt

@SimonSapin

This comment has been minimized.

Copy link
Member Author

commented Feb 25, 2017

{"status": "FAIL", "stack": null, "subtest": null, "action": "test_result", "test": "/_mozilla/css/overflow_wrap_a.html", "line": 64827, "message": "/_mozilla/css/overflow_wrap_a.html 57a783cf615a456badeb0f6c306887f9213b1ea0\n/_mozilla/css/overflow_wrap_ref.html 739aa7187d267c9ed72935ed31382631170de234\nTesting 57a783cf615a456badeb0f6c306887f9213b1ea0 == 739aa7187d267c9ed72935ed31382631170de234", "expected": "PASS"}

@bors-servo retry

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Feb 25, 2017

⌛️ Testing commit eacc12d with merge b7109ad...

bors-servo added a commit that referenced this pull request Feb 25, 2017
Update cssparser to 0.11

<!-- Please describe your changes on the following line: -->

<s>Depends on https://github.com/servo/rust-cssparser/pull/122.</s>

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/15735)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Feb 25, 2017

💔 Test failed - linux-rel-wpt

@SimonSapin

This comment has been minimized.

Copy link
Member Author

commented Feb 25, 2017

@bors-servo retry

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Feb 25, 2017

⚡️ Previous build results for android, arm32, arm64, linux-dev, mac-dev-unit, mac-rel-wpt1, windows-gnu-dev, windows-msvc-dev are reusable. Rebuilding only linux-rel-css, linux-rel-wpt, mac-rel-css, mac-rel-wpt2...

@SimonSapin

This comment has been minimized.

Copy link
Member Author

commented Feb 25, 2017

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Feb 25, 2017

💔 Test failed - linux-rel-wpt

SimonSapin added 3 commits Feb 26, 2017
The match_ignore_ascii_case! macro does ASCII-case-insensitive matching.
In cssparser verion 0.11, it will require its patterns to be already
lower case.
In cssparser version 0.11, this macro will stop implicitly borrowing its
own input.
@SimonSapin SimonSapin force-pushed the cssparserup branch from eacc12d to 2f996e3 Feb 26, 2017
@SimonSapin

This comment has been minimized.

Copy link
Member Author

commented Feb 26, 2017

I thought the failure above was intermittent because I couldn’t reproduce locally at first, but it looks like this is a real bug. I must have messed up my initial testing. The bug was in the "Use ascii_case_insensitive_phf_map! in PropertyId::parse" commit: I forgot to add property aliases to the map. The failing test is using the word-wrap property, an alias for overflow-wrap. With that fixed, the test passes on my machine.

@bors-servo r=emilio

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2017

📌 Commit 2f996e3 has been approved by emilio

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2017

⌛️ Testing commit 2f996e3 with merge 94e563e...

bors-servo added a commit that referenced this pull request Feb 26, 2017
Update cssparser to 0.11

<!-- Please describe your changes on the following line: -->

<s>Depends on https://github.com/servo/rust-cssparser/pull/122.</s>

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/15735)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2017

💡 This pull request was already approved, no need to approve it again.

  • This pull request is currently being tested. If there's no response from the continuous integration service, you may use retry to trigger a build again.
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2017

📌 Commit 2f996e3 has been approved by emilio

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2017

@bors-servo bors-servo merged commit 2f996e3 into master Feb 26, 2017
4 checks passed
4 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
dependency-ci Dependencies checked
Details
homu Test successful
Details
@emilio emilio deleted the cssparserup branch Feb 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.