[webkit-reviews] review granted: [Bug 231361] implement transform: perspective(none) : [Attachment 442480] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Nov 2 08:21:45 PDT 2021


Simon Fraser (smfr) <simon.fraser at apple.com> has granted Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 231361: implement transform: perspective(none)
https://bugs.webkit.org/show_bug.cgi?id=231361

Attachment 442480: Patch

https://bugs.webkit.org/attachment.cgi?id=442480&action=review




--- Comment #5 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 442480
  --> https://bugs.webkit.org/attachment.cgi?id=442480
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=442480&action=review

The calc issues are not new with this patch but if testing shows that calc is
broken here, please file a followup bug.

> Source/WebCore/css/TransformFunctions.cpp:325
> +	       Length perspectiveLength = Length(0, LengthType::Fixed);

Doesn't need to be initialized since every branch has an assignment.

> Source/WebCore/css/TransformFunctions.cpp:333
> +		   double doubleValue = firstValue.doubleValue();
> +		   ASSERT(doubleValue >= 0);

Do we reject negative values earlier, or will perspective(-100px) assert here?
We don't want assertions for any successfully parsed user input.

> Source/WebCore/css/TransformFunctions.cpp:334
> +		   perspectiveLength = 
Length(clampToPositiveInteger(doubleValue), LengthType::Fixed);

Integer is weird. Why can't perspective values be floating point? (not new).

Does this parsing work for calc()?

> Source/WebCore/css/parser/CSSPropertyParser.cpp:1843
> +    if (auto perspective = consumeNumberRaw(args, ValueRange::NonNegative))
{
> +	   transformValue->append(CSSPrimitiveValue::create(*perspective,
CSSUnitType::CSS_PX));
> +	   return true;
> +    }

Does this work with calc()?


More information about the webkit-reviews mailing list