dualfroz
19b1be8421
The ordered-list numbering used intval() on the "start" attribute inside a truthiness check, so a valid start="0" was treated as absent and the list restarted numbering at 1. Detect an explicit numeric start attribute with is_numeric() instead, so <ol start=\"0\"> renders as 0. 1. ...
74 lines
2.3 KiB
Markdown
74 lines
2.3 KiB
Markdown
# fix: honor start="0" on ordered lists
|
|
|
|
## Problem
|
|
|
|
Converting an ordered list whose `start` attribute is `0` produces the wrong
|
|
numbering: the list restarts at `1` instead of `0`.
|
|
|
|
```php
|
|
$converter = new League\HTMLToMarkdown\HtmlConverter();
|
|
$converter->convert('<ol start="0"><li>Item A</li><li>Item B</li></ol>');
|
|
// actual: "1. Item A\n2. Item B"
|
|
// expected: "0. Item A\n1. Item B"
|
|
```
|
|
|
|
`<ol start="0">` is valid HTML, and CommonMark supports ordered lists that begin
|
|
at `0`, so the resulting Markdown is incorrect. Any other start value (for
|
|
example `start="120"`) works.
|
|
|
|
## Root cause
|
|
|
|
`src/Converter/ListItemConverter.php:55`. The start value was read inside a
|
|
truthiness check:
|
|
|
|
```php
|
|
if ($listType === 'ol' && ($parent = $element->getParent()) && ($start = \intval($parent->getAttribute('start')))) {
|
|
$number = $start + $element->getSiblingPosition() - 1;
|
|
} else {
|
|
$number = $element->getSiblingPosition();
|
|
}
|
|
```
|
|
|
|
`\intval('0')` is `0`, which is falsy, so an explicit `start="0"` takes the
|
|
`else` branch and the list is renumbered from the sibling position (1-based).
|
|
The check cannot distinguish "start attribute absent" from "start attribute set
|
|
to 0", because `getAttribute('start')` returns `''` when the attribute is missing
|
|
and `intval('')` is also `0`.
|
|
|
|
## The fix
|
|
|
|
Detect an explicitly provided numeric `start` attribute with `\is_numeric()`
|
|
instead of relying on the truthiness of `intval()`:
|
|
|
|
```php
|
|
if ($listType === 'ol' && ($parent = $element->getParent()) && \is_numeric($start = $parent->getAttribute('start'))) {
|
|
$number = (int) $start + $element->getSiblingPosition() - 1;
|
|
} else {
|
|
$number = $element->getSiblingPosition();
|
|
}
|
|
```
|
|
|
|
`\is_numeric('')` is `false` (missing attribute -> normal numbering) while
|
|
`\is_numeric('0')` is `true`, so `start="0"` is now respected. Existing values
|
|
such as `start="120"` are unaffected.
|
|
|
|
## Test
|
|
|
|
Added an assertion in `HtmlConverterTest::testLists`:
|
|
|
|
```php
|
|
$this->assertHtmlGivesMarkdown('<ol start="0"><li>Item A</li><li>Item B</li></ol>', "0. Item A\n1. Item B");
|
|
```
|
|
|
|
## Gate
|
|
|
|
```
|
|
composer install && vendor/bin/phpunit --no-coverage
|
|
```
|
|
|
|
Result: exit code `0` - `OK (50 tests, 230 assertions)`.
|
|
|
|
Counterfactual: reverting `ListItemConverter` to the `intval()` truthiness check
|
|
fails the new assertion - `<ol start="0">` yields `1. Item A\n2. Item B` instead
|
|
of `0. Item A\n1. Item B`.
|