Page MenuHomePhabricator

Bug 1517511 - Simplify computed::LengthOrPercentage and friends. r=heycam
ClosedPublic

Authored by emilio on Jan 6 2019, 10:00 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 29, 11:06 AM
Unknown Object (File)
Fri, Apr 25, 7:02 PM
Unknown Object (File)
Sat, Apr 19, 7:02 AM
Unknown Object (File)
Thu, Apr 17, 2:20 AM
Unknown Object (File)
Feb 18 2025, 10:12 PM
Unknown Object (File)
Jan 3 2025, 11:18 AM
Unknown Object (File)
Dec 20 2024, 4:45 PM
Unknown Object (File)
Dec 15 2024, 6:56 PM
Subscribers
None

Details

Summary

This is a first step to share LengthOrPercentage representation between Rust and
Gecko.

We need to preserve whether the value came from a calc() expression, for now at
least, since we do different things depending on whether we're calc or not right
now. See https://github.com/w3c/csswg-drafts/issues/3482 and dependent bugs for
example.

That means that the gecko conversion code needs to handle calc() in a bit of an
awkward way until I change it to not be needed (patches for that incoming in the
next few weeks I hope).

I need to add a hack to exclude other things from the PartialEq implementation
because the new conversion code is less lossy than the old one, and we relied on
the lousiness in AnimationValue comparison (in order to start transitions and
such, in [1] for example).

I expect to remove that manual PartialEq implementation as soon as I'm done with
the conversion.

The less lossy conversion does fix a few serialization bugs for animation values
though, like not loosing 0% values in calc() when interpolating lengths and
percentages, see the two modified tests:

  • property-types.js
  • test_animation_properties.html

Diff Detail

Repository
rMOZILLACENTRAL mozilla-central
Lint
Lint Not Applicable
Unit
Tests Not Applicable
Build Status
Buildable 30799
Build 41444: arc lint + arc unit

Event Timeline

phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".Jan 6 2019, 10:01 PM
phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".
phab-bot removed a project: secure-revision.

Can you please add some WPTs for the calc-related behavior changes with this patch.

servo/components/style/values/animated/transform.rs
1169–1170

This is simple enough now that I'd probably just write fx.length_component().px() etc. below in the Translate3D case, and move the comment there too.

servo/components/style/values/computed/length.rs
176–177

Nit: Maybe s/raw/non-calc/ to be clearer?

341–342

This comment needs updating.

514

Can this be derived now?

543–544

Nit: swap these two lines to keep them sorted.

562–563

Yes. :-)

This revision is now accepted and ready to land.Jan 7 2019, 7:25 AM

The calc() behavior change is just during animation / interpolation really, and it's already tested by the property-types.js change.

servo/components/style/values/computed/length.rs
514

Yes, though I need to make LengthOrPercentageOrAuto a generic type for it to work, so I'll do it as a separate patch.

562–563

Same reason to do it in a separate patch :)

emilio marked 2 inline comments as done.

Revision updated.