Skip to content

BadMethodCallException on nested html lists on valid HTML #1265

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

samimussbach
Copy link

Description

We did not consider nested html lists as edge case in #1239.
Additionally the markup in the Sample 26 is invalid. When nesting lists the <ol> should be in a <li> not outside (compare https://www.w3schools.com/html/html_lists.asp).

I corrected the documentation an added a test which fails with

BadMethodCallException : Cannot add ListItemRun in ListItemRun. /opt/Development/PHPWord/src/PhpWord/Element/AbstractContainer.php:230 /opt/Development/PHPWord/src/PhpWord/Element/AbstractContainer.php:130 /opt/Development/PHPWord/src/PhpWord/Element/AbstractContainer.php:112 /opt/Development/PHPWord/src/PhpWord/Shared/Html.php:438 /opt/Development/PHPWord/src/PhpWord/Shared/Html.php:163 /opt/Development/PHPWord/src/PhpWord/Shared/Html.php:195 /opt/Development/PHPWord/src/PhpWord/Shared/Html.php:177 /opt/Development/PHPWord/src/PhpWord/Shared/Html.php:440 /opt/Development/PHPWord/src/PhpWord/Shared/Html.php:163 /opt/Development/PHPWord/src/PhpWord/Shared/Html.php:195 /opt/Development/PHPWord/src/PhpWord/Shared/Html.php:177 /opt/Development/PHPWord/src/PhpWord/Shared/Html.php:195 /opt/Development/PHPWord/src/PhpWord/Shared/Html.php:177 /opt/Development/PHPWord/src/PhpWord/Shared/Html.php:70 /opt/Development/PHPWord/tests/PhpWord/Shared/HtmlTest.php:296

Assertions in Test are not correct yet, I did not know how the markup would be.

I see this PR as WIP, I think you are much faster in tackling this than me, hopefully this PR helps a bit.

Checklist:

  • I have run composer check and no errors were reported
  • The new code is covered by unit tests
  • I have update the documentation to describe the changes

@troosan
Copy link
Contributor

troosan commented Feb 7, 2018

I think the correct HTML would be

<ol>
    <li><p style="font-weight: bold;">List 1 item 1</p></li>
    <li>List 1 item 2
        <ol>
            <li>sub list 1</li>
            <li>sub list 2</li>
        </ol>
    </li>
    <li>List 1 item 3</li>
</ol>

instead of

<ol>
    <li><p style="font-weight: bold;">List 1 item 1</p></li>
    <li>List 1 item 2</li>
    <li>
        <ol>
            <li>sub list 1</li>
            <li>sub list 2</li>
        </ol>
    </li>
    <li>List 1 item 3</li>
</ol>

@samimussbach
Copy link
Author

Yes absolutely. This is how I updated the sample and the test

@troosan
Copy link
Contributor

troosan commented Feb 7, 2018

sorry, I wrongly copied your html code.
The nested list should be part of the second list item, not added to an additional list item

@samimussbach
Copy link
Author

Your fix in #1273 works for us. Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants