Skip to content

Commit 1942442

Browse files
authored
Merge pull request #848 from Automattic/fix/847-css-recognition-cascade
Improve generic image crop recognition
2 parents 89e6586 + da4fbb8 commit 1942442

4 files changed

Lines changed: 307 additions & 42 deletions

File tree

.github/workflows/php-transformer.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,8 @@ jobs:
107107
WORDPRESS_TEST_DB: wordpress_site_plan_test
108108
run: |
109109
git clone --depth=1 --branch 6.7.4 https://github.com/WordPress/wordpress-develop.git "$WORDPRESS_DEVELOP_DIR"
110-
composer config --working-dir="$WORDPRESS_DEVELOP_DIR" --json policy.advisories.ignore-id '{"PKSA-mh9b-91zm-m1gy":{"on-audit":false,"reason":"Pinned WordPress 6.7.4 test runtime; WPCS is not loaded by the integration test."},"PKSA-rdkp-vv9z-mjkg":{"on-audit":false,"reason":"Pinned WordPress 6.7.4 test runtime; affected build tooling is not loaded by the integration test."},"PKSA-6vdd-n4sx-knhy":{"on-audit":false,"reason":"Pinned WordPress 6.7.4 test runtime; affected build tooling is not loaded by the integration test."}}'
110+
composer config --working-dir="$WORDPRESS_DEVELOP_DIR" --json policy.advisories.ignore-id '{"PKSA-mh9b-91zm-m1gy":{"on-audit":false,"reason":"Pinned WordPress 6.7.4 test runtime; WPCS is not loaded by the integration test."},"PKSA-kh6k-gs3g-dgr6":{"on-audit":false,"reason":"Pinned WordPress 6.7.4 test runtime; PHPCSUtils is not loaded by the integration test."},"PKSA-rdkp-vv9z-mjkg":{"on-audit":false,"reason":"Pinned WordPress 6.7.4 test runtime; affected build tooling is not loaded by the integration test."},"PKSA-6vdd-n4sx-knhy":{"on-audit":false,"reason":"Pinned WordPress 6.7.4 test runtime; affected build tooling is not loaded by the integration test."}}'
111+
composer require --working-dir="$WORDPRESS_DEVELOP_DIR" --dev phpcsstandards/phpcsutils:1.0.12 --no-update
111112
composer install --working-dir="$WORDPRESS_DEVELOP_DIR" --no-interaction --prefer-dist --no-progress
112113
for attempt in $(seq 1 30); do
113114
if mysql --host=127.0.0.1 --port=3306 --user=root --execute='SELECT 1'; then break; fi

php-transformer/src/HtmlToBlocks/HtmlTransformer.php

Lines changed: 84 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,9 @@ final class HtmlTransformer
364364
*/
365365
private array $conditionalStyleRules = array();
366366

367+
/** @var list<array{selector: string, property: string, value: string, conditions: list<string>, order: int}> Ordered crop declarations, including duplicates. */
368+
private array $imageShapeStyleRules = array();
369+
367370
/**
368371
* @var array<int, array{selector: string, pseudo: string, declarations: array<string, string>}>
369372
*/
@@ -634,6 +637,7 @@ public function transform(string $html, array $options = array()): TransformerRe
634637
$this->staticClassPromotions = $this->detectStaticClassPromotions($html);
635638
$this->staticStyleRules = $this->staticStyleRules($html, (string) ($options['static_css'] ?? ''));
636639
$this->conditionalStyleRules = $this->conditionalStyleRules($html, (string) ($options['static_css'] ?? ''));
640+
$this->imageShapeStyleRules = $this->imageShapeStyleRules($html, (string) ($options['static_css'] ?? ''));
637641
$this->staticPseudoElementStyleRules = $this->staticPseudoElementStyleRules($html, (string) ($options['static_css'] ?? ''));
638642
$this->cssCustomProperties = $this->cssCustomProperties($html, (string) ($options['static_css'] ?? ''));
639643
$this->resetPresentationResolutionCache();
@@ -11800,16 +11804,19 @@ private function imagePresentationAttributes(DOMElement $image, ?DOMElement $fig
1180011804
*/
1180111805
private function imageShapeConstraintAttributes(DOMElement $image): array
1180211806
{
11803-
$declarations = $this->structuralPresentationDeclarations($image);
11807+
$declarations = array(
11808+
'aspect-ratio' => $this->imageShapeDeclaration($image, 'aspect-ratio'),
11809+
'object-fit' => $this->imageShapeDeclaration($image, 'object-fit'),
11810+
);
1180411811
$aspectRatio = $this->normalizedAspectRatio(
11805-
$this->desktopViewportDeclaration($image, 'aspect-ratio', (string) ($declarations['aspect-ratio'] ?? ''))
11812+
(string) $declarations['aspect-ratio']
1180611813
);
1180711814
// `object-fit:cover !important` is a common defence against core's
1180811815
// `.wp-block-image img` rules. Strip importance symmetrically with
1180911816
// normalizedAspectRatio, or the keyword never matches the allowlist below
1181011817
// and the whole promotion silently declines.
1181111818
$scale = strtolower($this->cssValueWithoutImportant(
11812-
$this->desktopViewportDeclaration($image, 'object-fit', (string) ($declarations['object-fit'] ?? ''))
11819+
(string) $declarations['object-fit']
1181311820
));
1181411821

1181511822
if ( '' === $aspectRatio || ! in_array($scale, array( 'cover', 'contain' ), true) ) {
@@ -11822,57 +11829,65 @@ private function imageShapeConstraintAttributes(DOMElement $image): array
1182211829
);
1182311830
}
1182411831

11825-
/**
11826-
* WordPress `core/image` carries a SINGLE aspectRatio/scale, but authored CSS
11827-
* often makes these responsive: a base rule plus `@media (min-width: N)`
11828-
* overrides. The block must reflect what renders at the primary/desktop
11829-
* viewport, so among the base value and every matching min-width override we
11830-
* pick the value from the widest min-width breakpoint (<= the desktop
11831-
* reference width). max-width-bounded (mobile) media rules are ignored; the
11832-
* base value wins only when no qualifying min-width override declares it.
11833-
* Ties at one breakpoint go to the last rule, as the cascade decides them.
11834-
*/
11835-
private function desktopViewportDeclaration(DOMElement $element, string $property, string $baseValue): string
11832+
/** Resolve one crop declaration at the desktop viewport using the CSS cascade. */
11833+
private function imageShapeDeclaration(DOMElement $element, string $property): string
1183611834
{
11837-
$winningWidth = -1;
11838-
$winningValue = $baseValue;
11839-
// A declaration in the element's own `style` attribute outranks every
11840-
// matched stylesheet rule at normal importance, whatever viewport that
11841-
// rule is bound to. $baseValue already carries it.
11842-
$inlineValue = (string) ($this->cssDeclarations($this->attr($element, 'style'))[$property] ?? '');
11843-
$inlineOwns = '' !== trim($inlineValue);
11844-
$inlineImportant = $inlineOwns && $this->cssValueIsImportant($inlineValue);
11845-
11846-
foreach ( $this->conditionalStyleRules as $rule ) {
11847-
if ( ! isset($rule['declarations'][$property]) || ! isset($rule['conditions']) ) {
11835+
$winner = null;
11836+
foreach ($this->imageShapeStyleRules as $rule) {
11837+
if ($property !== $rule['property'] || ! $this->matchesCssSelector($element, $rule['selector'])) {
1184811838
continue;
1184911839
}
11850-
if ( $inlineOwns && ( $inlineImportant || ! $this->cssValueIsImportant((string) $rule['declarations'][$property]) ) ) {
11851-
continue;
11840+
if (array() !== $rule['conditions']) {
11841+
$minWidth = $this->conditionsDesktopMinWidth($rule['conditions']);
11842+
if (null === $minWidth || $minWidth > self::DESKTOP_REFERENCE_WIDTH) {
11843+
continue;
11844+
}
1185211845
}
11853-
if ( ! $this->matchesCssSelector($element, $rule['selector']) ) {
11854-
continue;
11846+
$candidate = array(
11847+
'value' => $rule['value'],
11848+
'specificity' => $this->mediaTextSelectorSpecificity($rule['selector']),
11849+
'order' => $rule['order'],
11850+
'inline' => false,
11851+
);
11852+
if ($this->imageShapeDeclarationWins($candidate, $winner)) {
11853+
$winner = $candidate;
1185511854
}
11856-
11857-
$minWidth = $this->conditionsDesktopMinWidth($rule['conditions']);
11858-
if ( null === $minWidth || $minWidth > self::DESKTOP_REFERENCE_WIDTH ) {
11855+
}
11856+
$inlineEntries = $this->imageShapeDeclarationEntries($this->attr($element, 'style'));
11857+
foreach ($inlineEntries as $index => $entry) {
11858+
if ($property !== $entry['property']) {
1185911859
continue;
1186011860
}
11861-
if ( $minWidth >= $winningWidth ) {
11862-
$winningWidth = $minWidth;
11863-
$winningValue = (string) $rule['declarations'][$property];
11861+
$candidate = array('value' => $entry['value'], 'specificity' => array(PHP_INT_MAX, PHP_INT_MAX, PHP_INT_MAX), 'order' => PHP_INT_MAX - count($inlineEntries) + $index, 'inline' => true);
11862+
if ($this->imageShapeDeclarationWins($candidate, $winner)) {
11863+
$winner = $candidate;
1186411864
}
1186511865
}
1186611866

11867-
return $winningValue;
11867+
return is_array($winner) ? $winner['value'] : '';
11868+
}
11869+
11870+
/** @param array{value:string,specificity:array{int,int,int},order:int,inline:bool} $candidate @param array{value:string,specificity:array{int,int,int},order:int,inline:bool}|null $current */
11871+
private function imageShapeDeclarationWins(array $candidate, ?array $current): bool
11872+
{
11873+
if (null === $current) {
11874+
return true;
11875+
}
11876+
$candidateImportant = $this->cssValueIsImportant($candidate['value']);
11877+
$currentImportant = $this->cssValueIsImportant($current['value']);
11878+
if ($candidateImportant !== $currentImportant) {
11879+
return $candidateImportant;
11880+
}
11881+
$specificity = $this->compareMediaTextSpecificity($candidate['specificity'], $current['specificity']);
11882+
return 0 < $specificity || (0 === $specificity && $candidate['order'] >= $current['order']);
1186811883
}
1186911884

1187011885
/**
1187111886
* The min-width (px) at which a conditional rule's `@media` prelude(s) begin
11872-
* to apply, or null when the rule is not a pure min-width desktop override:
11873-
* a non-`@media` condition, a negated query, a media type that is not the
11874-
* rendered screen, any `max-width` bound (mobile-capped range), or a media
11875-
* feature without a usable px min-width all disqualify it. Nested conditions
11887+
* to apply, or null when a condition cannot be positively evaluated at the
11888+
* desktop viewport. `@layer` is an unconditional grouping wrapper. A bounded
11889+
* set of modern crop-relevant `@supports` tests is accepted; unknown feature
11890+
* queries remain unresolved rather than being flattened. Nested conditions
1187611891
* must all qualify; the effective breakpoint is the widest.
1187711892
*
1187811893
* @param list<string> $conditions
@@ -11883,6 +11898,15 @@ private function conditionsDesktopMinWidth(array $conditions): ?int
1188311898

1188411899
foreach ( $conditions as $condition ) {
1188511900
$condition = trim($condition);
11901+
if ( 1 === preg_match('/^@layer\b/i', $condition) ) {
11902+
continue;
11903+
}
11904+
if ( 1 === preg_match('/^@supports\b/i', $condition) ) {
11905+
if ($this->supportsDesktopImageCropCondition($condition)) {
11906+
continue;
11907+
}
11908+
return null;
11909+
}
1188611910
if ( 1 !== preg_match('/^@media\b/i', $condition) ) {
1188711911
return null;
1188811912
}
@@ -11917,6 +11941,26 @@ private function conditionsDesktopMinWidth(array $conditions): ?int
1191711941
return $minWidth;
1191811942
}
1191911943

11944+
/** Bounded browser-capability facts used for crop rules under @supports. */
11945+
private function supportsDesktopImageCropCondition(string $condition): bool
11946+
{
11947+
$query = strtolower(trim(preg_replace('/^@supports\s*/i', '', $condition) ?? ''));
11948+
$query = preg_replace('/^\((.*)\)$/s', '$1', $query) ?? $query;
11949+
[$property, $value] = array_pad(array_map('trim', explode(':', $query, 2)), 2, '');
11950+
11951+
if ('display' === $property) {
11952+
return in_array($value, array('flex', 'grid', 'inline-flex', 'inline-grid'), true);
11953+
}
11954+
if ('aspect-ratio' === $property) {
11955+
return '' !== $this->normalizedAspectRatio($value);
11956+
}
11957+
if ('object-fit' === $property) {
11958+
return in_array($value, array('cover', 'contain'), true);
11959+
}
11960+
11961+
return false;
11962+
}
11963+
1192011964
private function cssValueWithoutImportant(string $value): string
1192111965
{
1192211966
return trim(preg_replace('/\s*!\s*important\s*$/i', '', $value) ?? $value);

php-transformer/src/HtmlToBlocks/Style/StyleResolutionTrait.php

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1065,7 +1065,7 @@ private function mergedPresentationStyle(DOMElement $element): string
10651065
}
10661066

10671067
$inlineStyle = $this->attr($element, 'style');
1068-
if ( array() === $this->staticStyleRules || ! $this->isHighValueStyledElement($element) ) {
1068+
if ( array() === $this->staticStyleRules || (! $this->isHighValueStyledElement($element) && ! $this->hasGenericRecognitionDemand($element)) ) {
10691069
$this->mergedPresentationStyleCache[$cacheKey] = $inlineStyle;
10701070
return $inlineStyle;
10711071
}
@@ -1135,6 +1135,25 @@ private function isHighValueStyledElement(DOMElement $element): bool
11351135
return $this->highValueStyleBoundaryPolicy()->matches($element);
11361136
}
11371137

1138+
/** Image crop recognition is structural and selector-driven, not name-driven. */
1139+
private function hasGenericRecognitionDemand(DOMElement $element): bool
1140+
{
1141+
if ('img' !== strtolower($element->tagName)) {
1142+
return false;
1143+
}
1144+
1145+
foreach (array_merge($this->staticStyleRules, $this->conditionalStyleRules) as $rule) {
1146+
if (! $this->matchesCssSelector($element, $rule['selector'])) {
1147+
continue;
1148+
}
1149+
if (array_intersect(array('aspect-ratio', 'object-fit', 'object-position'), array_keys($rule['declarations']))) {
1150+
return true;
1151+
}
1152+
}
1153+
1154+
return false;
1155+
}
1156+
11381157
/**
11391158
* @return array<int, array{selector: string, declarations: array<string, string>, mediaTextDeclarations: list<array{property: string, value: string, important: bool}>, mediaTextSpecificity: array{int, int, int}}>
11401159
*/
@@ -1192,6 +1211,96 @@ private function staticStyleRules(string $html, string $linkedCss): array
11921211
return $rules;
11931212
}
11941213

1214+
/**
1215+
* Preserve source order and duplicate declarations for image crop cascade
1216+
* resolution. General presentation maps intentionally collapse duplicates.
1217+
*
1218+
* @return list<array{selector: string, property: string, value: string, conditions: list<string>, order: int}>
1219+
*/
1220+
private function imageShapeStyleRules(string $html, string $linkedCss): array
1221+
{
1222+
$css = trim($linkedCss);
1223+
if (preg_match_all('@<style\b[^>]*>(.*?)</style>@is', $html, $matches)) {
1224+
$css .= ('' === $css ? '' : "\n") . implode("\n", array_map('trim', $matches[1]));
1225+
}
1226+
$rules = array();
1227+
$order = 0;
1228+
$this->collectImageShapeStyleRules(preg_replace('@/\*.*?\*/@s', '', $css) ?? $css, array(), $rules, $order);
1229+
1230+
return $rules;
1231+
}
1232+
1233+
/** @param list<string> $conditions @param list<array{selector: string, property: string, value: string, conditions: list<string>, order: int}> $rules */
1234+
private function collectImageShapeStyleRules(string $css, array $conditions, array &$rules, int &$order): void
1235+
{
1236+
$directCss = $css;
1237+
$events = array();
1238+
for ($offset = 0, $length = strlen($css); $offset < $length; ++$offset) {
1239+
if ('@' !== $css[$offset]) {
1240+
continue;
1241+
}
1242+
$blockStart = $this->findCssToken($css, '{', $offset);
1243+
$statementEnd = $this->findCssToken($css, ';', $offset);
1244+
if (null === $blockStart || (null !== $statementEnd && $statementEnd < $blockStart)) {
1245+
continue;
1246+
}
1247+
$end = $this->findMatchingCssBrace($css, $blockStart);
1248+
if (null === $end) {
1249+
continue;
1250+
}
1251+
$prelude = trim(substr($css, $offset, $blockStart - $offset));
1252+
$directCss = substr_replace($directCss, str_repeat(' ', $end - $offset + 1), $offset, $end - $offset + 1);
1253+
if (preg_match('/^@(media|container|supports|layer|scope|starting-style)\b/i', $prelude)) {
1254+
$events[] = array('offset' => $offset, 'css' => substr($css, $blockStart + 1, $end - $blockStart - 1), 'conditions' => array_merge($conditions, array($prelude)));
1255+
}
1256+
$offset = $end;
1257+
}
1258+
if (preg_match_all('/([^{}]+)\{([^{}]+)\}/', $directCss, $matches, PREG_SET_ORDER | PREG_OFFSET_CAPTURE)) {
1259+
foreach ($matches as $match) {
1260+
$events[] = array('offset' => $match[0][1], 'prelude' => $match[1][0], 'body' => $match[2][0], 'conditions' => $conditions);
1261+
}
1262+
}
1263+
usort($events, static fn (array $left, array $right): int => $left['offset'] <=> $right['offset']);
1264+
foreach ($events as $event) {
1265+
if (isset($event['css'])) {
1266+
$this->collectImageShapeStyleRules($event['css'], $event['conditions'], $rules, $order);
1267+
continue;
1268+
}
1269+
$entries = $this->imageShapeDeclarationEntries((string) $event['body']);
1270+
if (array() === $entries) {
1271+
continue;
1272+
}
1273+
foreach (explode(',', (string) $event['prelude']) as $selector) {
1274+
$selector = trim($selector);
1275+
if ('' === $selector || str_starts_with($selector, '@') || $this->selectorCarriesPseudoState($selector) || ! $this->isSupportedCssSelector($selector)) {
1276+
continue;
1277+
}
1278+
foreach ($entries as $entry) {
1279+
$rules[] = array('selector' => $selector, 'property' => $entry['property'], 'value' => $entry['value'], 'conditions' => $event['conditions'], 'order' => $order++);
1280+
}
1281+
}
1282+
}
1283+
}
1284+
1285+
/** @return list<array{property: string, value: string}> */
1286+
private function imageShapeDeclarationEntries(string $style): array
1287+
{
1288+
$entries = array();
1289+
foreach (CssValueSplitter::splitTopLevel($style, array(';')) as $declaration) {
1290+
if (! str_contains($declaration, ':')) {
1291+
continue;
1292+
}
1293+
[$property, $value] = array_map('trim', explode(':', $declaration, 2));
1294+
$property = strtolower($property);
1295+
$value = preg_replace('/\s+/', ' ', $value) ?? $value;
1296+
if (in_array($property, array('aspect-ratio', 'object-fit'), true) && '' !== $value) {
1297+
$entries[] = array('property' => $property, 'value' => $value);
1298+
}
1299+
}
1300+
1301+
return $entries;
1302+
}
1303+
11951304
/**
11961305
* Collect author rules nested in conditional at-rules. Their declarations
11971306
* must remain class-owned even though only the base cascade is available to

0 commit comments

Comments
 (0)