Skip to content

Don't put <p> around summary tag inside details with markdown flag set? #1079

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
fiendish opened this issue Nov 30, 2020 · 4 comments · Fixed by #1188
Closed

Don't put <p> around summary tag inside details with markdown flag set? #1079

fiendish opened this issue Nov 30, 2020 · 4 comments · Fixed by #1188
Labels
bug Bug report. confirmed Confirmed bug report or approved feature request. core Related to the core parser code. extension Related to one or more of the included extensions.

Comments

@fiendish
Copy link

The summary tag is a special tag inside details that I think should be ignored when processing internal markdown.

from markdown import markdown
from pprint import pprint

a = """
<details markdown="1">
<summary><b>Click to expand table</b></summary>

| Entity                              |   Count |
|:------------------------------------|--------:|
| Thing 1                      |      16 |
| Thing 2                      |       9 |

</details>
"""

pprint(markdown(a, extensions=["extra"]))
' <details>\n'
 '<p><summary><b>Click to expand table</b></summary></p>\n'
 '<table>\n'
 '<thead>\n'
 '<tr>\n'
 '<th align="left">Entity</th>\n'
 '<th align="right">Count</th>\n'
 '</tr>\n'
 '</thead>\n'
 '<tbody>\n'
 '<tr>\n'
 '<td align="left">Thing 1</td>\n'
 '<td align="right">16</td>\n'
 '</tr>\n'
 '<tr>\n'
 '<td align="left">Thing 2</td>\n'
 '<td align="right">9</td>\n'
 '</tr>\n'
 '</tbody>\n'
 '</table>\n'
 '</details>'

Note here that there are <p></p> around the summary, but IMO there should not be.

@waylan
Copy link
Member

waylan commented Nov 30, 2020

Can you point to support for this in the HTML5 spec?

Sent with GitHawk

@waylan waylan added the more-info-needed More information needs to be provided. label Dec 1, 2020
@fiendish
Copy link
Author

fiendish commented Dec 1, 2020

I don't understand the request. Support for what, the summary tag?
See: https://html.spec.whatwg.org/multipage/interactive-elements.html#the-summary-element
Also: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/summary

A <summary> element may only be used as the first child of a <details> element.

@facelessuser
Copy link
Collaborator

summary is being treated as inline from what I can tell. You can modify the elements treated as block in a plugin though, or you can modify the block elements of the Markdown instance directly: https://github.com/Python-Markdown/markdown/blob/master/markdown/core.py#L75. Now, could we treat summary as a block element by default? That is up to @waylan, but this can be worked around.

Alternatively, you could consider using a plugin for details: https://facelessuser.github.io/pymdown-extensions/extensions/details/.

@waylan
Copy link
Member

waylan commented Dec 1, 2020

Looking at the spec it seems that the summary tag is only valid as the first child of a detail tag and can only contain elements which a p tag can contain. Any other use would be invalid. Therefore...

  1. We don't need to consider uses outside of detail. If a document author uses it in that way that is on them. In other words we don't need to be concerned with using a different parsing behavior outside of the very limited valid use-case.
  2. Wrapping it in p would cause it to not be the first child of detail and therefore it would always be invalid to wrap it in a p tag.
  3. As it can not contain any block level elements, we should only parse its content as span-level content.

Given the above, yes, summary should be added to the default list of block-level elements. It should also be added to the list of span_tags of the md_in_html extension to control the default behavior when it is given a markdown="1" attribute.

@waylan waylan added bug Bug report. confirmed Confirmed bug report or approved feature request. core Related to the core parser code. extension Related to one or more of the included extensions. and removed more-info-needed More information needs to be provided. labels Dec 1, 2020
waylan added a commit to waylan/markdown that referenced this issue Nov 3, 2021
waylan added a commit that referenced this issue Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug report. confirmed Confirmed bug report or approved feature request. core Related to the core parser code. extension Related to one or more of the included extensions.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants