Skip to content
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

Add .logical to an auto-inserted div.para if it contains a block element #2026

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

siefkenj
Copy link
Contributor

Currently when

<ul>
  <li>
    <p><ul><li>foo</li></ul></p>
  </li>
</ul>

is processed, the resulting p turns into a div.para.logical. However,

<ul>
  <li>
    <ul><li>foo</li></ul>
  </li>
</ul>

gets an auto-inserted p which turns to a div.para even though it contains a block element. This PR makes the behavior of the auto-inserted p consistent.

@rbeezer
Copy link
Collaborator

rbeezer commented Jul 25, 2023

Thanks @siefkenj. Good catch. It'll be a few days before I can merge.

@davidfarmer: do you expect CSS ramifications? Would a beta be useful or necessary?

@rbeezer
Copy link
Collaborator

rbeezer commented Aug 2, 2023

OK, put this through testing on the sample article.

  • Only changes seem to be when the li content is a cd. Which may illustrate a shortage of examples.
  • Nothing harmful for CSS to fix, it appears.
  • I believe an ol,ul, dl cannot appear as a child if anli without an intervening p. Or said differently, the list structure must be a child of a p. So could be removed from the @test?

Shouldn't these "unstructured" li be exploded into several HTML p? I wonder if the pre-processor should just wrap the content of an "unstrcuctured" li into a p and then it will be handled appropriately in all conversions (especially HTML)?

@siefkenj
Copy link
Contributor Author

siefkenj commented Aug 3, 2023

I am not sure what @test refers to. And since we do not use any <p> tags anywhere in HTML, I don't think exploding is necessary.

Yes, moving this step to the preprocessor would be an option (that's what I do in the JS version. I first add a <p> around anything that is not allowed as a direct child via the schema; of course you'll want to take special care for the <md>/<m> tags, as you pointed out to me)

@rbeezer
Copy link
Collaborator

rbeezer commented Aug 3, 2023

I am not sure what @test refers to

 <xsl:if test="ol|ul|dl|me|men|md|mdn|cd">

I don't think ol|ul|dl are necessary as it never happens this way.

@siefkenj
Copy link
Contributor Author

siefkenj commented Aug 4, 2023

Are you saying that the case

<ul>
  <li>
    <ul><li>foo</li></ul>
  </li>
</ul>

is handled elsewhere in the code?

@rbeezer
Copy link
Collaborator

rbeezer commented Aug 4, 2023

Your interior ul has to live in a p, according to the schema. Who knows how permissive the conversions are.

Some exceptions, such as GOAL-LIKE, presumes an interior list not in a p.

@siefkenj
Copy link
Contributor Author

siefkenj commented Aug 4, 2023

The ul is current auto-wrapped in a p as per line 5919.

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

Successfully merging this pull request may close these issues.

2 participants