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

Fix issue of UTF-8 belows the autolinked email address. #463

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

david50407
Copy link

See #388 for more details.

@jstewart
Copy link

@vmg - Any chance of seeing this merged? #388 is biting me pretty hard, and and this PR fixes that bug.

Reproduce:

renderer = Redcarpet::Render::HTML.new(with_toc_data: true)
md = Redcarpet::Markdown.new(renderer, no_intra_emphasis: true, tables: true, autolink: true, quote: true)
md.render("“foo@example.com“")

# => "<p>“<a href=\"mailto:foo@example.com%E2\">foo@example.com\xE2</a>\x80\x9C</p>\n"
# irb(main):008:0> md.render("“foo@example.com“").valid_encoding?
# => false

As you can see, the first quote is rendered properly but the second is not.

@david50407
Copy link
Author

maybe ping @robin850 ...?

kaorukobo added a commit to kaorukobo/redcarpet that referenced this pull request Feb 18, 2022
kaorukobo added a commit to kaorukobo/redcarpet that referenced this pull request Feb 18, 2022
@@ -126,6 +126,12 @@ def test_whitespace_after_urls
assert_equal exp, rd
end

def test_auto_linked_email_utf8_issue
rd = render_with({ autolink: true }, "a@b.c」\nd@example.coü")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part fails on the latest source at 03b664d. So it should be:

rd = render("a@b.c」\nd@example.coü", with: [:autolink])
exp = %{<p><a href="mailto:a@b.c">a@b.c</a>」\n<a href="mailto:d@example.co">d@example.co</a>ü</p>}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OMG this project is still alive!!

Got it, I will rebase this PR soon. Thanks for reviewing. :)

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

Successfully merging this pull request may close these issues.

None yet

3 participants