Didn't want the work (and review comments) in #15364 to go wasted. Fixes#15313
Source-Repo: https://github.com/servo/servo
Source-Revision: e2aecc3ed7c66a49f2baf13c16e56c3fb6c990cd
<!-- Please describe your changes on the following line: -->
- Position parse no longer accepts invalid three value positions
---
<!-- 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
- [X] These changes fix#15488 .
<!-- Either: -->
- [X] There are tests for these changes OR
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Source-Repo: https://github.com/servo/servo
Source-Revision: 8c4931f26f49cf328b63880b6b9b9bf8d99279c4
<!-- Please describe your changes on the following line: -->
This is a WIP PR. Just HorizontalPosition / VerticalPosition implementations are complete. I would like to get early feedbacks about this architecture. Here's some architectural topics to consider:
- I created `HorizontalPosition` and `VerticalPosition` structs for this and used them in `Position` as well. We have decided to split `Keyword` enum, but we need them as unified for `PositionComponent` enum. So I didn't split but I can split it if we prefer to change PositionComponent as well.
- If we prefer Keyword enum like this, we can create a SubPosition(or something like this) instead of HorizontalPosition/VerticalPosition enums since only difference is 2 lines in `parse` functions. We can create a `parse_horizontal` and `parse_vertical` instead and a lot of code duplication can be cleared.
- I couldn't find a good way to use HorizontalPosition/VerticalPosition's parse functions in `Position`'s parse function. It is a bit more complicated. I'm open to suggestions :)
- I don't know much about logical keywords so do I need to do something different? I placed some comments where logical keywords are processing.
Any advice about these?
---
<!-- 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
- [X] These changes fix#14458 (github issue number if applicable).
<!-- Either: -->
- [X] There are tests for these changes
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Source-Repo: https://github.com/servo/servo
Source-Revision: 5357f05ff785ee160d6b07de5d0e10aba199e578
<!-- Please describe your changes on the following line: -->
This changes the `parse` function's signature to include `ParserContext`, so that we don't introduce another trait just for the sake of the context. Instead, we can safely ignore the context whenever we don't need it.
---
<!-- 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
<!-- Either: -->
- [x] These changes do not require tests because it's a refactor
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
r? @SimonSapin or @emilio or @Manishearth
Source-Repo: https://github.com/servo/servo
Source-Revision: 4755cb7586ab4a89f35bbccf8b57c85ed2f428e7
<!-- Please describe your changes on the following line: -->
Moved parse functions to a Parse trait in style. r? @Manishearth
---
<!-- 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
- [X] These changes fix#14101 (github issue number if applicable).
<!-- Either: -->
- [x] There are tests for these changes OR
- [X] These changes do not require tests because new functionalities weren't added
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Source-Repo: https://github.com/servo/servo
Source-Revision: 6adae66f9c7ff58e0e31f3913471e575910a5861
<!-- Please describe your changes on the following line: -->
This will allow types to be generic over our local `ToCss`
---
<!-- 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
- [x] `./mach test-tidy` does not report any errors
<!-- Either: -->
- [x] These changes do not require tests because it's a refactor
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Source-Repo: https://github.com/servo/servo
Source-Revision: f48b3fe2197a59b29bc711fb1b5496cbb97bd330
<!-- Please describe your changes on the following line: -->
This is first part of fix#12690 and covers just specified style part for now.
r? @Manishearth
---
- [X] These changes fix#12690 (github issue number if applicable).
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Source-Repo: https://github.com/servo/servo
Source-Revision: b8d725d2079945a18c9b40250a2a722379dad966
Still WIP: I still need to use this somewhere and make serialization minimal. I'm not sure if I should do either in this PR.
The only other browser that handles basic shapes doesn't serialize correctly either (https://bugzilla.mozilla.org/show_bug.cgi?id=1290864), so that's not something we need to get done now.
As far as using this somewhere, I have the following options:
- Merge this now, work on using it in stylo in a followup.
- Just write extensive unit tests for parsing/serialization for now (stylo in a followup)
- Use this for clip-path in stylo only (which I intend to do anyway, just not sure if I should do it in this PR)
- Use this for clip-path in Servo (I'd rather not do this; this would be a huge change requiring a lot more layout knowledge than I currently have)
Thoughts? Review? @SimonSapin @bholley
Source-Repo: https://github.com/servo/servo
Source-Revision: 0fc0db67c64bf8ab4593d1f84b8d1be517fa1b74
