"perspective" property must reject negative values

[upsuper]

Spec: https://drafts.csswg.org/css-transforms-2/#perspective-property

(The spec says positive, but it should be non-negative... which is an issue of the spec.)

Relevant code: https://github.com/servo/servo/blob/master/components/style/properties/longhand/box.mako.rs#L1642-L1648

(Should be an easy one, but not clear what is the best way to fix. cc @Manishearth)

[BorisChiou]

The perspective matrix uses the parameter, d, to compute the matrix [1]: "-1/d", so zero should be unacceptable. However, IIRC, in Gecko, we treat prespective(0) as prespective(inf), so we still works on zero.

[1] https://drafts.csswg.org/css-transforms-2/#PerspectiveDefined

[upsuper]

We treat perspective(0) as perspective(inf), but we treat perspective: 0 as perspective: epsilon. Anyway, that happens in later place, so style system doesn't need to handle that. It just needs to reject negative value.

[BorisChiou]

I see. Thanks for clarification.

[highfive]

Hi! If you have any questions regarding this issue, feel free to make a comment here, or ask it in the #servo channel in IRC.

If you intend to work on this issue, then add @highfive: assign me to your comment, and I'll assign this to you. 😄

[Manishearth]

We just need to add parse_non_negative as the predefined type parse function there.

[ioctaptceb]

@highfive: assign me!

[highfive]

Hey @ioctaptceb! Thanks for your interest in working on this issue. It's now assigned to you!

[dcci]

Sorry if I'm missing something obvious, but what's the policy for PR timeouts? If nobody is actually working on this I'd like to try implementing it.

[jdm]

It is good form to ask the person directly before starting to work on somethng. @ioctaptceb have you made any progress?

[dcci]

ping?

[upsuper]

It seems @ioctaptceb doesn't respond after a whole week... This issue is now yours, @dcci :)

[ioctaptceb]

@dcci please go ahead!

[dcci]

I was unsure how to test this one, so I asked on IRC.
Copying for the archives:

14:32 <@SimonSapin> davide: then add a testharness.js test in tests/wpt/mozilla/tests/css
14:32 <@SimonSapin> see tests/wpt/README.md for instructions
14:34 <@SimonSapin> use a style="…" attribute in HTML with an invalid value, and assert in JS that element.style.perspective is the empty string
14:36 <@SimonSapin> also try assigning a different value to
                    element.style.perspective, like assigning " 1Px" and see it
                    become "1px" when accessed again (which goes through the
                    parser and serializer)

[KiChjang]

@dcci What's the status of this?

[n0max]

@highfive: assign me

[highfive]

Hey @n0max! Thanks for your interest in working on this issue. It's now assigned to you!

[highfive]

cc @emilio

[wafflespeanut]

@n0max Did you start working on this? Need help?

[n0max]

I have already changed the parsing to non negative and created some test cases.
But the mozresource.pyc file was added to the MANIFEST.json and running ./mach update-manifest don't change anything. I'm currently try to fix this.
My changes are at commit.

[wafflespeanut]

Ah, just revert the manifest to the previous commit, update-manifest again and amend your changes to your current commit?

[jdm]

You will need to remove the the mozresource.pyc file before running update-manifest again.

[n0max]

Thank you, that worked.