Closed Bug 1573830 Opened 5 years ago Closed 2 years ago

We incorrectly skip 0%s in transform serialization.


(Core :: CSS Parsing and Computation, defect, P3)




106 Branch
Tracking Status
firefox-esr91 --- wontfix
firefox-esr102 --- wontfix
firefox104 --- wontfix
firefox105 --- wontfix
firefox106 --- fixed


(Reporter: emilio, Assigned: aw4910819, Mentored)




(Keywords: regression, Whiteboard: [lang=rust], [wptsync upstream])


(1 file)

Priority: -- → P3
Regressed by: 1532134
Keywords: regression

Just noticing that this same issue seems to affect the translate:; property as well as transform: translate();

/* input CSS */
one value {
  translate: 1px;

two values {
  translate: 1px 0;

/* parsed in CSSOM */
one value {
  translate: 1px;
two values {
  translate: 1px;

This is present in Firefox Nightly 73.0b2 (64-bit) on MacOS, though also in every version of Firefox I check as well so I don't believe it's specific to this build. The behaviour seems the same as when transform: translate(1px, 0) is parsed.

This is about 0%. Skipping 0px is correct, including for the individual transform, see

It does the same behaviour for unitless 0 and 0% as well:

/* input */
unitless { translate: 1px 0; }
percent { translate: 1px 0%; }

/* parsed in CSSOM */
unitless { translate: 1px; }
percent { translate: 1px; }

Ok, sure... Yeah, it affects all the properties where we do skipping-if-zero... But comment 0 didn't have any percent :-)

I was going to tag this as a good-first-bug, but Chromium agrees with us in translate... So a bit trickier.

Anyhow, to fix this, we should replace the "Zero::is_zero" here (which right now calls is_definitely_zero, which returns true for 0%) for something like LengthPercentage::is_zero_length, which should do something like !has_percentage && is_definitely_zero or such. May need to define that method in a trait that applies both to the specified and computed LengthPercentage.

That does it for transform: translate(). We should also change the y.is_zero() call here for a call to the same method to affect the translate property.

A simple test-case is evaluating this on the terminal: = "translate(1px, 0%)" // Should be "translate(1px, 0%)"

Then we'd need to convert that in an automated test or what not.

Mentor: emilio
Whiteboard: [lang=rust]

I'd be interested in taking this on as a first bug, but I'm unsure how to approach it from a workflow standpoint. Unless I'm mistaken, it looks like the test cases for CSS serialization like this are in servo/tests/unit/style/parsing/, but these tests have dependencies that are checked into Servo but not checked into mozilla-central. I would assume that means development on this is expected to take place in the Servo repo, since it's not possible to submit a patch to mozilla-central that you can demonstrate will pass these tests. But then why is this a Bugzilla bug and not a Github issue?

Apologies if there's some doc I missed explaining what the relationship between these two repos and their respective workflows is besides someone occasionally merging a subtree of Servo into mozilla-central.

Flags: needinfo?(emilio)

No problem. Those tests are not run in mozilla-central (and we've just been deleting those unit tests when they get in the way, really).

We prefer integration tests for that, and there a lot of them, both in wpt (see for example, and in the internal mochitests, like all the tests that use property_database.js.

Flags: needinfo?(emilio)
Has Regression Range: --- → yes

Hi, I'd do this as a first bug if no one else is working on it.
I've prepared a fix and I'm wondering who I should set as the reviewer.

Flags: needinfo?(emilio)

Sure, please go ahead and submit it with r=emilio, I can take a look at the patch :)

Flags: needinfo?(emilio)

Adds trait ZeroNoPercent to check for values that are 0 (such as 0px) but not 0%
: Enter commit message. Lines beginning with 'HG:' are removed.

Assignee: nobody → aw4910819
Pushed by
0% values are not skipped when parsing CSS transform. r=emilio
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 106 Branch
Created web-platform-tests PR for changes under testing/web-platform/tests
Whiteboard: [lang=rust] → [lang=rust], [wptsync upstream]
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.