commonmark-spec icon indicating copy to clipboard operation
commonmark-spec copied to clipboard

CJK link processing causes Markdown links and HTML links to have different URLs

Open ptmkenny opened this issue 4 years ago • 12 comments

The Commonmark spec currently results in the Japanese text in Markdown links getting converted. This is causing problems for me because I have Markdown links and HTML links on the same page, and my pa11y accessibility tests are failing with the Markdown links because they expect the Japanese text but instead are getting the converted text.

[Link](https://www.example.com/テレビ)

<a href="https://www.example.com/テレビ">Link</a>

This results in the following HTML:

<p><a href="https://www.example.com/%E3%83%86%E3%83%AC%E3%83%93">Link</a></p>

<p><a href="https://www.example.com/テレビ">Link</a></p>

In this case, I do not think the text should be converted. It's perfectly valid HTML to leave the links "as is", so no additional processing should be applied.

ptmkenny avatar Dec 14 '20 12:12 ptmkenny

The link from markdown still works though, right? You can open both and they go to the same place. Sounds like an issue in pa11y to me

wooorm avatar Dec 14 '20 13:12 wooorm

The links are equivalent, but there should not be reason to munge the URL to a different encoding (and code style) if it is valid as-is.

alerque avatar Dec 14 '20 13:12 alerque

Right, both URLs resolve to the same page, but it complicates any post processing that might be done after markdown.

For example, if I have a static site and I grep for a Japanese link, now I have to search for two patterns, the original Japanese and the converted one.

ptmkenny avatar Dec 14 '20 13:12 ptmkenny

This isn't an issue with the spec, because the spec doesn't mandate that the URLs be escaped this way. That's up to the renderer.

jgm avatar Dec 14 '20 20:12 jgm

According to Babelmark3, CM implementations disagree on this:

  • commonmark-java and flexmark-java do not percent-encode either URL
  • commonmark.js, league/commonmark, markdig, MD4C and pulldown-cmark encode the Markdown URL only (as described above); markdown-it safifies HTML beforehand
  • GFM encodes both URLs

Crissov avatar Dec 15 '20 10:12 Crissov

Yes, this is just an issue which the spec leaves to implementations. (The spec is really concerned with parsing, not rendering. It's a bit awkward that we use HTML for the examples/tests, for this reason; the examples inevitably have to make decisions on rendering that go beyond what the spec demands.)

jgm avatar Dec 15 '20 15:12 jgm

@jgm I believe the idea of leaving that to compilers/generators/implementations is not in the spec, it would be very useful in my opinion to have that documented

wooorm avatar Dec 15 '20 15:12 wooorm

The spec doesn't say "URLs must be encoded." It just says how to recognize something as a link destination, etc. But yes, we could add a note about this.

jgm avatar Dec 15 '20 16:12 jgm

My that was a reference to your earlier comment (“The spec is really concerned with parsing, not rendering. It's a bit awkward that we use HTML for the examples/tests, for this reason; the examples inevitably have to make decisions on rendering that go beyond what the spec demands.”), not with this specific issue of whether to encode URLs. I believe I’ve heard you voice that idea several times on this issue tracker, and I think it’d be good to add it to the spec, because to me the spec currently reads as dictating that certain input MUST lead to certain output without any room to diverge.

wooorm avatar Dec 15 '20 16:12 wooorm

Agreed. Perhaps add a sentence or two to this paragraph:

Since this document describes how Markdown is to be parsed into an abstract syntax tree, it would have made sense to use an abstract representation of the syntax tree instead of HTML. But HTML is capable of representing the structural distinctions we need to make, and the choice of HTML for the tests makes it possible to run the tests against an implementation without writing an abstract syntax tree renderer.

jgm avatar Dec 15 '20 18:12 jgm

Let me know what you think about the text added in the above commit.

jgm avatar Dec 16 '20 00:12 jgm

Sorry. I think this looks goods! 👍

wooorm avatar Feb 05 '21 15:02 wooorm