Skip to content

admonitions (note and warn) are not correctly rendered on github #68

Description

@stefanfoulis

With rst it is easy to make admonitions for notes and warnings:

.. warning:: don't do this

Using github-markup to render this sort of thing correctly creates this output:

<div class="warning">
    <p class="first admonition-title">Warning</p>
    <p class="last">don't do this</p>
</div>

On github it seems this is just rendered as two normal <p> tags (In gists, the source browser and README.rst).

<p>Warning</p>
<p>don't do this</p>

And that turns out to look like this:


github current admonition rendering


This makes admonitions useless. Is github doing some extra post-processing of the output? If so, can it be modified to allow this construct to get through.

Just copying the correct code into the browser on a github page in firebug revealed, that stylesheets for this already exist (although they are not that pretty):


github current admonition rendering if the markup were correct


Something similar to the styling on readthedocs.org would be cool:


readthedocs admonition rendering


Activity

  1. chickenkiller commented on Jul 16, 2011

    @chickenkiller

    The same kind of issues appear for some asciidoc specific renderings which use classes. The root cause is that a HTML sanitization step is done, and among other things, removes class names.

    Maybe we should find a way to detect some common classe names per format and give them to Markup so that they are considered harmless, and could also be unified (e.g. Warning, Note are common cases that happen to exist in several formats) so that a CSS style defined by Markup (or higher level layers such as Gollum?) can be applied to them.

  2. rvanlaar commented on Dec 21, 2012

    @rvanlaar

    .. note :: is still not supported. I'ld this issue to be reopend. See also #72

  3. beniwohli commented on Mar 27, 2013

    @beniwohli

    Common @github, this shouldn't be to hard and would be a huge improvement for rendering README's and the like.

  4. mgaitan commented on May 10, 2013

    @mgaitan

    any progress on this?

  5. mojavelinux commented on May 10, 2013

    @mojavelinux
    Contributor

    AsciiDoc has the same problem. It would be helpful if:

    1. there are a set of styles classes in the GitHub stylesheet to use (perhaps there are, but they need to be pointed out)
    2. the style classes from the rendered HTML aren't stripped out so that the admonition blocks actually work
  6. mojavelinux commented on May 10, 2013

    @mojavelinux
    Contributor

    Actually, I found these styles in the second stylesheet loaded:

    #readme.rst div.admonition,#readme.rst div.attention,#readme.rst div.caution,#readme.rst div.danger,#readme.rst div.error,#readme.rst div.hint,#readme.rst div.important,#readme.rst div.note,#readme.rst div.tip,#readme.rst div.warning{
    margin:2em;border:medium outset;padding:1em
    }
    #readme.rst div.admonition p.admonition-title,#readme.rst div.hint p.admonition-title,#readme.rst div.important p.admonition-title,#readme.rst div.note p.admonition-title,#readme.rst div.tip p.admonition-title{
    font-weight:bold;font-family:sans-serif
    }
    #readme.rst div.attention p.admonition-title,#readme.rst div.caution p.admonition-title,#readme.rst div.danger p.admonition-title,#readme.rst div.error p.admonition-title,#readme.rst div.warning p.admonition-title{
    color:red;font-weight:bold;font-family:sans-serif
    }

    I'm glad they are there, but why make them specific to rst? Why not a standard way of doing admonitions so that any language that has them can use them?

  7. mojavelinux commented on May 10, 2013

    @mojavelinux
    Contributor

    Ah, I see now that @stefanfoulis already figured out that these styles are there using firebug hacking. As it turns out, they are ugly as sin.

  8. dbrgn commented on Oct 8, 2013

    @dbrgn

    Please fix this. Warnings are commonly used in README files...

  9. matthijskooijman commented on Nov 4, 2013

    @matthijskooijman

    Hm, just bumped into this issue as well. Without proper support for admonitions, I can't properly write my protocol documentation and I'll have to look for some other way to render them :-S

    As for the suggested fix: a whitelist of classnames could be made, but why are CSS class stripped in the first place? To prevent messing up the github layout?

  10. matthijskooijman commented on Nov 5, 2013

    @matthijskooijman

    @github @gjtorikian Would it help if I tried to distill a list of classes that docutils can possibly use in the generated HTML? If I can do anything else to get his issue fixed sooner rather than later, let me know.

  11. gjtorikian commented on Nov 5, 2013

    @gjtorikian
    Contributor

    They're intentionally being stripped out. It happens in comments here, too:

    Warning

    don't do this

    We have a pretty strict whitelist of what can come through, as defined here: https://github.com/jch/html-pipeline/blob/master/lib/html/pipeline/sanitization_filter.rb

    Part of this is done for security reasons. Another part is dealing with supporting class names for multiple markups.

    I'm on the fence about closing this as a "won't fix." It's interesting that there are styles in place (probably from years ago). There are actually several RST styles there (for tables and images). And, in general, I'm kind of in favor of showing some light admonition rendering. I'll have to dig around for the rationale and reasoning for stripping out class names from markup. It's not super high on my "list of shit to do" but I can at least guarantee I'll look into it. Hope this helps.

  12. matthijskooijman commented on Nov 5, 2013

    @matthijskooijman

    I was going to argue that writing raw HTML in a comment is different from the rendered HTML, but in the end both of them are user controlled. The actual HTML parts generated by the markup processor could possibly be safe, but since most markup languages have features to include raw HTML as well, there is no way to tell the difference, and sanitizing the generated HTML is the only sensible route.

    Stripping class names as part of the sanitizing suprises me a bit, but I can imagine that there sane arguments for that as well.

    I wonder if docutils and/or other markup processors would have some configuration options to make them emit a single standardized set of classes, instead of each their own. No time to look into that right now, though.

    Thanks for looking into this at any rate, it helps :-)

  13. mojavelinux commented on Nov 5, 2013

    @mojavelinux
    Contributor

    I was going to argue that writing raw HTML in a comment is different from the rendered HTML, but in the end both of them are user controlled. The actual HTML parts generated by the markup processor could possibly be safe, but since most markup languages have features to include raw HTML as well, there is no way to tell the difference, and sanitizing the generated HTML is the only sensible route.

    AsciiDoc is aware of when it is outputting built-in HTML vs user-generated "pass through" HTML. I would expect that the built-in HTML would be trusted, at least enough to keep the CSS classes around. By stripping the CSS classes, it becomes impossible to distinguish between the different block types, admonitions in particular.

    What would be nice is if the GitHub stylesheet would offer a list of high-level CSS classes for basic styling of established structures and then require the markup processors to produce HTML that conforms to those HTML structures. I can certainly make that happen in AsciiDoc using the custom Asciidoctor templates.

    The problem so far has been that we haven't received any indication from GitHub that customizing the output of the processor would be an investment that would lead to any action. GitHub, if you give us requirements, then we can work to meet them. But without any information, we're all just standing around hoping for something to happen.

  14. mitar commented on Jan 10, 2014

    @mitar

    +1

  15. added a commit that references this issue on Apr 1, 2014
  16. 7 remaining items

  17. gene1wood commented on Jun 13, 2014

    @gene1wood

    +1

  18. brifordwylie commented on Jun 28, 2014

    @brifordwylie

    +1 supporting a least some subset of admonitions would be super.

  19. qrilka commented on Jul 17, 2014

    @qrilka

    +1

  20. itsjohncs commented on Aug 1, 2014

    @itsjohncs

    +1

  21. tautrimas commented on Aug 31, 2014

    @tautrimas

    +1

  22. kaycebasques commented on Sep 5, 2014

    @kaycebasques

    +1

  23. lukemarsden commented on Sep 7, 2014

    @lukemarsden

    +1

  24. sunbit commented on Oct 23, 2014

    @sunbit

    +1

  25. geoffroy-aubry commented on Nov 2, 2014

    @geoffroy-aubry

    +1

  26. yvaucher commented on Nov 11, 2014

    @yvaucher

    +1

  27. ErwinRieger commented on Dec 5, 2014

    @ErwinRieger

    +1

  28. Orcomp commented on Dec 10, 2014

    @Orcomp

    +1

    Can we overcome this by creating fictional languages? (i.e. Note and Warning)

    '''Warning
    Put some warning in here
    '''

    '''Note
    Put some notes here
    '''

    Purposely did not use back quotes to show the idea.

  29. bkeepers commented on Dec 18, 2014

    @bkeepers
    Contributor

    Thanks everyone for letting us know this is important to you. I would love this feature as well. However this cannot be implemented in this library, so I'm going to close this issue. If you'd like to voice support for this feature, [email protected] keeps a tally of feature requests.

  30. locked and limited conversation to collaborators on Dec 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions