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

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

Open
ptmkenny opened this issue Dec 14, 2020 · 12 comments

Comments

@ptmkenny
Copy link

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.

@wooorm
Copy link
Contributor

wooorm commented Dec 14, 2020

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

@alerque
Copy link

alerque commented Dec 14, 2020

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.

@ptmkenny
Copy link
Author

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.

@jgm
Copy link
Member

jgm commented Dec 14, 2020

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.

@Crissov
Copy link
Contributor

Crissov commented Dec 15, 2020

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

@jgm
Copy link
Member

jgm commented Dec 15, 2020

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.)

@wooorm
Copy link
Contributor

wooorm commented Dec 15, 2020

@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

@jgm
Copy link
Member

jgm commented Dec 15, 2020

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.

@wooorm
Copy link
Contributor

wooorm commented Dec 15, 2020

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.

@jgm
Copy link
Member

jgm commented Dec 15, 2020

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 added a commit that referenced this issue Dec 16, 2020
@jgm
Copy link
Member

jgm commented Dec 16, 2020

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

@wooorm
Copy link
Contributor

wooorm commented Feb 5, 2021

Sorry. I think this looks goods! 👍

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

5 participants