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

AsciiDoc block titles not properly rendered #479

Closed
nc7s opened this issue Apr 2, 2015 · 20 comments
Closed

AsciiDoc block titles not properly rendered #479

nc7s opened this issue Apr 2, 2015 · 20 comments

Comments

@nc7s
Copy link

nc7s commented Apr 2, 2015

As in following text

.list title
- item 1
- item 2
- item 3

In GitHub repos .list title is rendered as a normal paragraph, unlike rendered by AsciiDoctor CLI. Browser DevTool checked it's a div without class="title" like the CLI rendered one does.

Not sure what this attributes list does but it seems a bit suspicious(read source, didn't find anything related). Will anyone be willing to explain it for me? Thanks a lot.

@mojavelinux
Copy link
Contributor

All CSS information is stripped from the generated HTML by the sanitization filter on GitHub. A possible workaround is to wrap the title in <strong> or <em> in the output so that that style hint makes it through. For that, though, we need to make it a bit easier in the Asciidoctor backend by making the generation of the title centralized. That way, it can be changed with one small tweak. Best to file that issue in Asciidoctor core.

@mojavelinux
Copy link
Contributor

I'm open to other suggestions about how to get the title to render nicely. We could also consider using an <h6> element. Or perhaps a <caption> element. We may have to get creative.

@nc7s
Copy link
Author

nc7s commented Apr 2, 2015

All CSS information including class attribute of tags? That's a bit strange for me.

@nc7s nc7s changed the title AsciiDoc list titles not properly rendered AsciiDoc block titles not properly rendered Apr 2, 2015
@mojavelinux
Copy link
Contributor

Yep. C'est la vie.

@nc7s
Copy link
Author

nc7s commented Apr 7, 2015

The original asciidoc.py behaves the same. I'm not quite clear now whether it's a problem of AsciiDoc or GitHub. Maybe GitHub should change their sanitization rules a little?

@elextr
Copy link

elextr commented Apr 16, 2015

@mojavelinux can asciidoctor tell its providing input to github? Presumably you wouldn't want to change the normal HTML output being styled using class.

@mojavelinux
Copy link
Contributor

We're thinking about changing the title of a block to the <strong> tag or <h6>...something that will be styled by default without CSS applied.

@elextr
Copy link

elextr commented Apr 16, 2015

I guess if the tag has a class on it the existing css will override the tag styling so existing docs won't change?

@powerman
Copy link

I've related, but a bit wider question: there are a lot of limitations (and even probably some bugs) in GitHub rendering of Asciidoc. I've noticed them while trying to create Asciidoc cheatsheet for GitHub. So, I had to cut my cheatsheet to small subset of markup which works ok, but you can check complete cheatsheet - it's full of // comments about rendering issues. Also, even markup which works ok for files has some issues when used in wiki - check "NOTE" block at beginning of this page.

So, the question is: where and how I should report these issues? Open issues here (one big or one per each rendering issue?), or write to [email protected] as mentioned in your CONTRIBUTING document for "Styling issues on GitHub", or in asciidoctor's repo (but I believe 95% of these issues is GitHub specific and not a bugs in asciidoctor) or no one is going to fix this anyway so I should just relax and leave it as is?

@mojavelinux
Copy link
Contributor

I guess if the tag has a class on it

...then it will be stripped out by the GitHub pipeline :/

there are a lot of limitations (and even probably some bugs) in GitHub rendering of Asciidoc

It's important to emphasize that this has always been the case, and is the case for most languages other than Markdown (you can see a lot of related issues filed for reStructuredText, for instance).

The process you proposed for filing issues describes the current state. We can fix things in core if they are a core problem, but otherwise we have to chip away at improving the integration via negotiation with the GitHub.com team. One way we have control to improve things is via the AsciiDoc html-pipeline filter, which is used on GitHub.com. (see https://github.com/asciidoctor/html-pipeline-asciidoc_filter). We can massage the content there to improve how it displays on GitHub. However, keep in mind that even the most semantic HTML is still going to have certain limitations that are true even for Markdown. The rendering on GitHub is intended to be somewhat generic since it's not a full document publishing solution. In short, it's intended to look rather plain (and thus simple).

One way we've solved this issue is by providing an alternate viewer for documents on GitHub named docgist (http://gist.asciidoctor.org). This shows documents as rendered by Asciidoctor without any filtering. It's a nice lightweight way to get styling we will likely never get on GitHub.

To summarize, yes, we should continue working with GitHub to improve the display of AsciiDoc. There will always be compromises, but we want to do our best to make the look the best it can be given the constraints. It's a process.

@mojavelinux
Copy link
Contributor

For this issue, the question is whether block titles should be wrapped in <strong>, <h6> or something else?

We can apply this change using the pipeline filter (the easiest way) or by modifying the HTML output of Asciidoctor (a bit harder due to how titles are currently handled by the converter). Either way, we can get titles to display in bold on GitHub without GitHub having to change anything.

Either way, I think we should move this issue to https://github.com/asciidoctor/html-pipeline-asciidoc_filter.

@nc7s
Copy link
Author

nc7s commented May 2, 2015

Yep, it's now clearly not an issue of github/markup.

I thought once about , but you know, it's semanticly for only.

Being optional, block titles might fit better in than

.

Also since you @mojavelinux have opened asciidoctor/html-pipeline-asciidoc_filter#1, is it possible to utilize name attribute?

@powerman
Copy link

powerman commented May 2, 2015

is it possible to utilize name attribute?

I've already manually tested it by adding markup like this:

+++<a name="..."></a>+++

and it works fine (i.e. it wasn't killed by github sanitizer filter). But, as was pointed in asciidoc maillist, the name attribute is obsolete in html5, so asciidoc/asciidoctor won't use it by default in place of <div id>, and only way to use it on github is with help of https://github.com/asciidoctor/html-pipeline-asciidoc_filter.

Being optional, block titles might fit better in <strong> than <h6>.

It looks like <h6> doesn't used right now for anything else (in asciidoc), while <strong> is used for *bold* markup. And looks like <h6> is rendered nice enough for block titles, with just one issue: non-zero bottom margin. I suppose sanitizer won't allow us to use <h6 style="margin-bottom:0">… so <div><strong> may looks better because of this.

@nc7s
Copy link
Author

nc7s commented May 2, 2015

seems a good way. Since nothing else uses that, it should be safe to add a line of CSS to remove its margin.

@mojavelinux
Copy link
Contributor

But, as was pointed in asciidoc maillist, the name attribute is obsolete in html5, so asciidoc/asciidoctor won't use it by default in place of

That's where the filter might be right choice since we'd be introducing markup specific for GitHub. Another possibility is that we create ids using the "user-content-" prefix that will allow them to still work on GitHub. This is documented here: http://opensoul.org/2014/09/05/dom-clobbering/

@mojavelinux
Copy link
Contributor

so <div><strong> may looks better because of this.

I tried it with the GitHub styles and this looks much better than <h6>. Using <strong> is also more true to the indented styling of a block title, which is meant to be a lead-in not a section heading.

@mojavelinux
Copy link
Contributor

I've filed an issue in the AsciiDoc filter for html-pipeline. See asciidoctor/html-pipeline-asciidoc_filter#2

@kivikakk
Copy link
Contributor

Seeing as this has gone a bit stale, I'll close this issue.

Just for some context, though; we disallow class attributes because the app itself uses classes in its own JavaScript to script the UI, and it'd be dangerous to let users potentially collide with those on the page. It'd also be very easy to completely break the layout of the page, which is something we try to avoid.

(I'm happy to try to get further traction on this particular issue, though! Let me know if there's more I can do from my end.)

@mojavelinux
Copy link
Contributor

Although this issue is may be stale, the upstream issue is still active. See asciidoctor/asciidoctor#2006.

The problem is, I still don't understand which gem is used where on GitHub. I'm helping to maintain two gems:

If I add the converter customization to html-pipeline-asciidoc_filter, I don't know if that's going to get used. I need more insight. Why does GitHub have two gems?

In the meantime, what I've considered doing to solve this issue is add conditional code into the core HTML converter to wrap the block title in a <strong> tag when rendering on GitHub (I know I'm on GitHub by the env-github attribute).

Another approach would be to have predefined CSS classes on specific tags that markup implementations like Asciidoctor could rely on. There are certain features that can only be provided by CSS, yet we completely lose the ability to provide them on GitHub. We're not talking about classes add by the user, but by the converter itself.

@kivikakk
Copy link
Contributor

As far as I'm aware, there's only one gem being used by GitHub — github-markup. I've looked grepped the entire history of the main GitHub app and of the Markup gem and neither have ever referred to html-pipeline-asciidoc_filter; likewise, I can't see any internal issues or PRs that refer to the the latter gem, which it looks like you've been maintaining.

I've only been working on HTML pipeline stuff for the last ~9 months so I don't have all the context, but do you have any record or log of us actually integrating and using html-pipeline-asciidoc_filter?

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

No branches or pull requests

6 participants
@kivikakk @mojavelinux @elextr @powerman @nc7s and others