Bug 217320

<rdar://problem/69956248>

Created attachment 410525 [details] Patch

<rdar://problem/69956248>

Created attachment 410527 [details] Patch

Comment on attachment 410527 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=410527&action=review > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:658 > + if (!rendererCanBeTransformed(renderer) || !rotate) swap these? > Source/WebCore/css/CSSValueKeywords.in:1143 > +// x > +// y Typically you'd leave these ones because they were already present, and they come first in the file (and comment the rotate versions) > Source/WebCore/css/TransformFunctions.cpp:476 > + return RotateTransformOperation::create(x, y, z, angle, type); Are the x, y, z values normalized after parsing? > Source/WebCore/css/parser/CSSPropertyParser.cpp:2141 > + RefPtr<CSSValue> parsedValue; This can go inside the loop. > Source/WebCore/css/parser/CSSPropertyParser.cpp:2168 > + if (parsedValue) { > + // If we've encountered an axis identifier, then this valus is invalid. > + if (axisIdentifier) > + return nullptr; > + list->append(*parsedValue); > + } else { > + // Then, attempt to parse an angle. We try this as a fallback rather than the first option because > + // a unitless 0 angle would be consumed as an angle. > + parsedValue = consumeAngle(range, cssParserMode, UnitlessQuirk::Forbid); > + if (parsedValue) { > + // If we had already parsed an angle or numbers but not 3 in a row, this value is invalid. > + if (angle || (list->length() && list->length() != 3)) > + return nullptr; > + angle = parsedValue; > + } else { > + // Finally, attempt to parse one of the axis identifiers. > + parsedValue = consumeIdent<CSSValueX, CSSValueY, CSSValueZ>(range); > + // If we failed to find one of those identifiers or one was already specified, or we'd previously > + // encountered numbers to specify a rotation axis, then this value is invalid. > + if (!parsedValue || axisIdentifier || list->length()) > + return nullptr; > + axisIdentifier = parsedValue; > + } > + } These might read easier with "continue" statements rather than "else". e.g. // 1. Attempt to parse a number. parsedValue = consumeNumber(); if (parsedValue) { // If we've encountered an axis... if (axisIdentifier) return nullptr; list->append(*parsedValue); continue; } // 2. Attempt to parse an angle....

Committed r267985: <https://trac.webkit.org/changeset/267985>

(In reply to Dean Jackson from comment #5) > Comment on attachment 410527 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=410527&action=review > > > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:658 > > + if (!rendererCanBeTransformed(renderer) || !rotate) > > swap these? Fixed in commit. > > Source/WebCore/css/CSSValueKeywords.in:1143 > > +// x > > +// y > > Typically you'd leave these ones because they were already present, and they > come first in the file (and comment the rotate versions) From the ChangeLog: Define the "z" value for the "rotate" property and ensure the "x" and "y" values are defined as well since -webkit-scroll-snap-type may not be enabled. > > Source/WebCore/css/TransformFunctions.cpp:476 > > + return RotateTransformOperation::create(x, y, z, angle, type); > > Are the x, y, z values normalized after parsing? I'm not sure what you mean there. The x/y/z values are just defined as <number> in CSS. > > Source/WebCore/css/parser/CSSPropertyParser.cpp:2141 > > + RefPtr<CSSValue> parsedValue; > > This can go inside the loop. Fixed in commit. > > Source/WebCore/css/parser/CSSPropertyParser.cpp:2168 > > + if (parsedValue) { > > + // If we've encountered an axis identifier, then this valus is invalid. > > + if (axisIdentifier) > > + return nullptr; > > + list->append(*parsedValue); > > + } else { > > + // Then, attempt to parse an angle. We try this as a fallback rather than the first option because > > + // a unitless 0 angle would be consumed as an angle. > > + parsedValue = consumeAngle(range, cssParserMode, UnitlessQuirk::Forbid); > > + if (parsedValue) { > > + // If we had already parsed an angle or numbers but not 3 in a row, this value is invalid. > > + if (angle || (list->length() && list->length() != 3)) > > + return nullptr; > > + angle = parsedValue; > > + } else { > > + // Finally, attempt to parse one of the axis identifiers. > > + parsedValue = consumeIdent<CSSValueX, CSSValueY, CSSValueZ>(range); > > + // If we failed to find one of those identifiers or one was already specified, or we'd previously > > + // encountered numbers to specify a rotation axis, then this value is invalid. > > + if (!parsedValue || axisIdentifier || list->length()) > > + return nullptr; > > + axisIdentifier = parsedValue; > > + } > > + } > > These might read easier with "continue" statements rather than "else". Used `continue` in the commit.