From b3451ba9fe8a3f6a92538a276527e199dc7e4cc6 Mon Sep 17 00:00:00 2001 From: David Grudl Date: Fri, 31 Jul 2026 19:19:10 +0200 Subject: [PATCH 1/2] HtmlHelpers: contenteditable, draggable, spellcheck & hidden are not boolean attributes (BC break) None of them is a boolean attribute in HTML; they are enumerated ones, so rendering them as a bare name was wrong in three separate ways: - a string 'false' is truthy in PHP, so contenteditable={$s} rendered the attribute present, i.e. the exact opposite of what was asked for - the empty value is not a keyword of draggable, it maps to the invalid value default 'auto', so draggable=true could not be expressed at all - 'attribute missing' and 'attribute false' are different states; the false value is what disables editing inside a contenteditable region, and dropping the attribute silently inherited from the parent instead Two new categories replace them. 'tristate' (contenteditable, draggable, spellcheck, writingsuggestions) maps bool to the true/false keywords, exactly like aria-* already does, so formatAriaAttribute now delegates to it. The 'valuedBool' one (hidden, popover) stays a flag but keeps a non-empty value, so hidden="until-found" survives. Falsy values are tested first there: hidden with a '0' coming from a database must not render hidden="0", which is an invalid value and would map back to the hidden state. --- docs/internals/escaping.md | 10 ++- src/Latte/Essential/Nodes/NAttrNode.php | 2 +- src/Latte/Runtime/HtmlHelpers.php | 60 +++++++++++++---- tests/common/Engine.migrationWarnings.phpt | 12 ++++ .../HtmlHelpers.formatTristateAttribute.phpt | 60 +++++++++++++++++ ...HtmlHelpers.formatValuedBoolAttribute.phpt | 43 +++++++++++++ tests/tags/n-attr.formatHtmlAttribute.phpt | 43 +++++++++++++ tests/tags/print.attribute.phpt | 64 +++++++++++++++++++ 8 files changed, 279 insertions(+), 15 deletions(-) create mode 100644 tests/runtime/HtmlHelpers.formatTristateAttribute.phpt create mode 100644 tests/runtime/HtmlHelpers.formatValuedBoolAttribute.phpt diff --git a/docs/internals/escaping.md b/docs/internals/escaping.md index 6ec4cb2cb..25802a9f0 100644 --- a/docs/internals/escaping.md +++ b/docs/internals/escaping.md @@ -54,7 +54,7 @@ element's `restoreEscape()` and leak the context past the element. A whole HTML attribute generated from an expression (`Html\ExpressionAttributeNode`) is not escaped via `Escaper` at all. The node picks a `Runtime\HtmlHelpers::format*Attribute` method at compile time — the attribute *type* -(`bool`/`list`/`data`/`aria`/`style`) comes from the attribute name via +(`bool`/`tristate`/`valuedBool`/`list`/`data`/`aria`/`style`) comes from the attribute name via `classifyAttributeType`, or is forced by an attribute modifier: `|toggle` anywhere in the chain, `|json` only as the **last** modifier (`json` never comes from the attribute name). Any non-HTML content type falls back to @@ -62,6 +62,14 @@ attribute name). Any non-HTML content type falls back to JSON smart-quoting), so they are a **third place** the escaping contract lives — audit them alongside the two layers above. +The categories are not interchangeable, because HTML resolves an *invalid* value +differently per attribute. `tristate` renders a bool as the `true`/`false` keyword, +but `valuedBool` must not: `hidden="false"` maps to the hidden state, so `false` there +has to drop the attribute instead. For the same reason `formatValuedBoolAttribute` +tests falsiness **before** treating the value as a keyword — narrowing that to +`is_string($value)` looks tidier but renders `hidden="0"` for a `'0'` coming from a +database, and the element silently disappears. + ## `ContentType` and contextual filters `ContentType` is **not a PHP enum** — it is a `final class` of string constants diff --git a/src/Latte/Essential/Nodes/NAttrNode.php b/src/Latte/Essential/Nodes/NAttrNode.php index 6d3a1c651..a7568082a 100644 --- a/src/Latte/Essential/Nodes/NAttrNode.php +++ b/src/Latte/Essential/Nodes/NAttrNode.php @@ -68,7 +68,7 @@ public static function formatHtmlAttribute(mixed $name, mixed $value, bool $migr { LR\HtmlHelpers::validateAttributeName($name); $type = LR\HtmlHelpers::classifyAttributeType($name); - if ($value === null || ($value === false && $type !== 'data' && $type !== 'aria')) { + if ($value === null || ($value === false && $type !== 'data' && $type !== 'aria' && $type !== 'tristate')) { return ''; } elseif ($value === true && $type === '') { return $name; diff --git a/src/Latte/Runtime/HtmlHelpers.php b/src/Latte/Runtime/HtmlHelpers.php index 11905b24b..94d57186f 100644 --- a/src/Latte/Runtime/HtmlHelpers.php +++ b/src/Latte/Runtime/HtmlHelpers.php @@ -22,10 +22,19 @@ final class HtmlHelpers { private const BooleanAttributes = [ 'allowfullscreen' => 1, 'async' => 1, 'autofocus' => 1, 'autoplay' => 1, 'checked' => 1, 'controls' => 1, - 'contenteditable' => 1, 'default' => 1, 'defer' => 1, 'disabled' => 1, 'draggable' => 1, 'formnovalidate' => 1, - 'hidden' => 1, 'inert' => 1, 'ismap' => 1, 'itemscope' => 1, 'loop' => 1, 'multiple' => 1, 'muted' => 1, - 'nomodule' => 1, 'novalidate' => 1, 'open' => 1, 'playsinline' => 1, 'readonly' => 1, 'required' => 1, - 'reversed' => 1, 'selected' => 1, 'spellcheck' => 1, + 'default' => 1, 'defer' => 1, 'disabled' => 1, 'formnovalidate' => 1, 'inert' => 1, 'ismap' => 1, + 'itemscope' => 1, 'loop' => 1, 'multiple' => 1, 'muted' => 1, 'nomodule' => 1, 'novalidate' => 1, + 'open' => 1, 'playsinline' => 1, 'readonly' => 1, 'required' => 1, 'reversed' => 1, 'selected' => 1, + ]; + + /** enumerated attributes whose keywords are 'true' and 'false' */ + private const TristateAttributes = [ + 'contenteditable' => 1, 'draggable' => 1, 'spellcheck' => 1, 'writingsuggestions' => 1, + ]; + + /** boolean-like attributes accepting an optional keyword, e.g. hidden="until-found" */ + private const ValuedBooleanAttributes = [ + 'hidden' => 1, 'popover' => 1, ]; private const SpaceSeparatedAttributes = [ @@ -166,13 +175,15 @@ public static function convertHtmlToText(string $s): string /** - * Returns the type category of an HTML attribute: 'bool', 'list', 'data', 'aria', 'style', or ''. + * Returns the type category of an HTML attribute: 'bool', 'tristate', 'valuedBool', 'list', 'data', 'aria', 'style', or ''. */ public static function classifyAttributeType(string $name): string { $name = strtolower($name); return match (true) { isset(self::BooleanAttributes[$name]) => 'bool', + isset(self::TristateAttributes[$name]) => 'tristate', + isset(self::ValuedBooleanAttributes[$name]) => 'valuedBool', isset(self::SpaceSeparatedAttributes[$name]) => 'list', str_starts_with($name, 'data-') => 'data', str_starts_with($name, 'aria-') => 'aria', @@ -211,6 +222,34 @@ public static function formatBoolAttribute(string $namePart, mixed $value): stri } + /** + * Formats HTML attribute whose keywords are 'true' and 'false'. + */ + public static function formatTristateAttribute(string $namePart, mixed $value, bool $migrationWarnings = false): string + { + if ($migrationWarnings && is_bool($value)) { + self::triggerMigrationWarning(trim($namePart), $value); + } + return match (true) { + is_bool($value) => $namePart . '="' . ($value ? 'true' : 'false') . '"', + default => self::formatAttribute($namePart, $value), + }; + } + + + /** + * Formats boolean HTML attribute accepting an optional keyword. + */ + public static function formatValuedBoolAttribute(string $namePart, mixed $value): string + { + return match (true) { + !$value => '', // falsy values are not keywords; hidden="0" would map back to the hidden state + $value === true => $namePart, + default => self::formatAttribute($namePart, $value), + }; + } + + /** * Formats space separated HTML attribute. */ @@ -268,14 +307,9 @@ public static function formatJsonAttribute(string $namePart, mixed $value): stri */ public static function formatAriaAttribute(string $namePart, mixed $value, bool $migrationWarnings = false): string { - if ($migrationWarnings && is_bool($value)) { - self::triggerMigrationWarning(trim($namePart), $value); - } - return match (true) { - is_bool($value) => $namePart . '="' . ($value ? 'true' : 'false') . '"', - is_array($value) => self::formatArrayAttribute($namePart, $value, fn($v, $k) => $v === true ? $k : $v, ' '), - default => self::formatAttribute($namePart, $value), - }; + return is_array($value) + ? self::formatArrayAttribute($namePart, $value, fn($v, $k) => $v === true ? $k : $v, ' ') + : self::formatTristateAttribute($namePart, $value, $migrationWarnings); } diff --git a/tests/common/Engine.migrationWarnings.phpt b/tests/common/Engine.migrationWarnings.phpt index a0c903fad..28b096480 100644 --- a/tests/common/Engine.migrationWarnings.phpt +++ b/tests/common/Engine.migrationWarnings.phpt @@ -33,6 +33,12 @@ Assert::error( 'Behavior change for attribute \'aria-foo\' with value true: previously it rendered as aria-foo="1", now it renders as aria-foo="true" (on line 1 at column 8)', ); +Assert::error( + fn() => $latte->renderToString('
'), + E_USER_WARNING, + 'Behavior change for attribute \'contenteditable\' with value false: previously it rendered as contenteditable="", now it renders as contenteditable="false" (on line 1 at column 25)', +); + // |accept Assert::same( '', @@ -50,3 +56,9 @@ Assert::same( '', $latte->renderToString(''), ); + +// toggle overrides the category derived from the attribute name +Assert::same( + '
', + $latte->renderToString('