Preserve units in computed Angle #16674
Conversation
Using specified Angle was wrong because it can keep calc value and serialize with calc(). Also that will allow us to pass angle unit later.
|
Heads up! This PR modifies the following files:
|
|
|
| /// Sets Angle value to this nsCSSValue. | ||
| pub fn set_angle(&mut self, angle: Angle) { | ||
| let (value, unit) = angle.to_gecko_values(); | ||
| self.mUnit = unit; |
There was a problem hiding this comment.
Oh, one thing I missed, please assert here we're not leaking anything (that our unit isn't one of the heap ones).
There was a problem hiding this comment.
(Not the unit we're setting, but the pre-existing unit)
|
Fixed that. That assert was your point, right? |
|
How is that assert ever going to be true? We definitely expect to go from I mean to assert it's not one of the actual units that needs freeing stuff in |
There was a problem hiding this comment.
r=me, assuming it holds, thanks!
We should refactor those methods to make clear when we call them, but I think that's part of https://bugzilla.mozilla.org/show_bug.cgi?id=1361032
|
|
||
| /// Sets Angle value to this nsCSSValue. | ||
| pub fn set_angle(&mut self, angle: Angle) { | ||
| debug_assert!(self.mUnit == nsCSSUnit::eCSSUnit_Null); |
|
@bors-servo r=emilio |
|
📌 Commit f8710bc has been approved by |
highfive
added
S-awaiting-merge
Preserve units in computed Angle <!-- Please describe your changes on the following line: --> It was converting all angles to radians before. But other browsers preserves the angle units. Fixed that behavior. --- <!-- 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 #16594 and [Bug 1360659](https://bugzilla.mozilla.org/show_bug.cgi?id=1360659) <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- 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/16674) <!-- Reviewable:end -->
|
☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev |
highfive
removed
the
S-awaiting-merge
It was converting all angles to radians before. But other browsers preserves the angle units. Fixed that behavior.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is