This is a bug to implement perspective(none), that is, the "none" keyword as an argument to the perspective transform function. This was agreed by the CSS Working Group in https://github.com/w3c/csswg-drafts/issues/6488 and added to the spec in https://github.com/w3c/csswg-drafts/pull/6691 . It was implemented in Gecko at https://bugzilla.mozilla.org/show_bug.cgi?id=1725207 and will be implemented in Chromium at https://bugs.chromium.org/p/chromium/issues/detail?id=1253596 . There are currently a few tests in WPT in: css/css-transforms/animation/transform-interpolation-001.html css/css-transforms/parsing/transform-valid.html and I plan to add one more in: css/css-transforms/parsing/transform-computed.html
<rdar://problem/84124161>
Created attachment 442367 [details] Patch
Created attachment 442471 [details] Patch
Created attachment 442480 [details] Patch
Comment on attachment 442480 [details] 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()?
Created attachment 443207 [details] Patch
(In reply to Simon Fraser (smfr) from comment #5) > Comment on attachment 442480 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=442480&action=review Thanks for the review. After chatting with you yesterday, I've modified this change to use std::optional<Length> for storing the perspective value. > The calc issues are not new with this patch but if testing shows that calc > is broken here, please file a followup bug. I've filed a followup for this at: https://bugs.webkit.org/show_bug.cgi?id=232669 > > > Source/WebCore/css/TransformFunctions.cpp:325 > > + Length perspectiveLength = Length(0, LengthType::Fixed); > > Doesn't need to be initialized since every branch has an assignment. I've reworked this code block a bit and now this is fixed. > > 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. I've removed the assertion. It's unclear if we can get negative values as a result of a calc here. > > 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()? I think there are indeed some issues with this part of the code. I've filed a new bug to investigate them. calc() does seem to work (there is a WPT test for it), but I think maybe the correct context isn't passed in here. I suspect that it's broken for certain values. > > 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()? The test is still passing, but I think there's more work to do here.
Committed r285255 (243866@main): <https://commits.webkit.org/243866@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 443207 [details].