fix: honor start="0" on ordered lists
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. ...
This commit is contained in:
1 parent
e017c9ceda
commit
19b1be8421
3 files changed
+76
-2
No files matched your search
@@ -0,0 +1,73 @@
|
||||
# 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`.
|
||||
@@ -52,8 +52,8 @@ class ListItemConverter implements ConverterInterface, ConfigurationAwareInterfa
|
||||
return $prefix . $this->listItemStyle . ' ' . $value . "\n";
|
||||
}
|
||||
|
||||
if ($listType === 'ol' && ($parent = $element->getParent()) && ($start = \intval($parent->getAttribute('start')))) {
|
||||
$number = $start + $element->getSiblingPosition() - 1;
|
||||
if ($listType === 'ol' && ($parent = $element->getParent()) && \is_numeric($start = $parent->getAttribute('start'))) {
|
||||
$number = (int) $start + $element->getSiblingPosition() - 1;
|
||||
} else {
|
||||
$number = $element->getSiblingPosition();
|
||||
}
|
||||
|
||||
@@ -192,6 +192,7 @@ class HtmlConverterTest extends TestCase
|
||||
$this->assertHtmlGivesMarkdown('<ol><li> Item A</li><li> Item B</li></ol>', "1. Item A\n2. Item B");
|
||||
$this->assertHtmlGivesMarkdown('<ol><li> <h3> Item A</h3><p>Description</p></li><li> Item B</li></ol>', "1. ### Item A\n \n Description\n2. Item B");
|
||||
$this->assertHtmlGivesMarkdown('<ol start="120"><li>Item A</li><li>Item B</li></ol>', "120. Item A\n121. Item B");
|
||||
$this->assertHtmlGivesMarkdown('<ol start="0"><li>Item A</li><li>Item B</li></ol>', "0. Item A\n1. Item B");
|
||||
$this->assertHtmlGivesMarkdown('<ul><li>first item of first list</li><li>second item of first list</li></ul><ul><li>first item of second list</li></ul>', "- first item of first list\n- second item of first list\n\n* first item of second list", ['list_item_style_alternate' => '*']);
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user