MarkdownTextBlock throws an ArgumentException when using a markdown link with a superscript link as text

[Sergio0694]

I'm submitting a...

• Bug report (I searched for similar issues and did not find one)

Current behavior

Trying to render a markdown like this throws an ArgumentException:

[^(www.google.com)](www.google.com)

The crash happens at the last line, localContext.InlineCollection.Add(link);, from this method: https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/a722f171cac7959d8fb39d732c5951018a2e8c07/Microsoft.Toolkit.Uwp.UI.Controls/MarkdownTextBlock/Render/MarkdownRenderer.Inlines.cs#L188

This is probably related in some way to #2802

Expected behavior

The markdown should be correctly rendered as a superscript link.

Minimal reproduction of the problem with instructions

Write the following text in a MarkdownTextBlock control:

[^(www.google.com)](www.google.com)

Environment

Nuget Package(s): Microsoft.Toolkit.Uwp.UI.Controls

Package Version(s): 5.0.0.0

Windows 10 Build Number:
- [ ] Fall Creators Update (16299)
- [ ] April 2018 Update (17134)
- [X] October 2018 Update (17763)
- [ ] Insider Build (build number: )

App min and target version:
- [ ] Fall Creators Update (16299)
- [ ] April 2018 Update (17134)
- [X] October 2018 Update (17763)
- [ ] Insider Build (xxxxx)

Device form factor:
- [X] Desktop
- [ ] Mobile
- [ ] Xbox
- [ ] Surface Hub
- [ ] IoT

Visual Studio 
- [X] 2017 (version: )
- [ ] 2017 Preview (version: )

[michael-hawker]

Thanks @Sergio0694, Is the ^(...) superscript notation standardized anywhere? I don't see GitHub supporting it.

[Sergio0694]

Hi @michael-hawker - it's not supported on GitHub, but it's part of Reddit's markdown (docs here).
It's present in the sample markdown in the UWP Toolkit sample app as well.

I do agree that having a superscript link inside a markdown link is a bit too much (found this while randomly looking at some markdown examples), but I think it'd be great if this was either added, or if the control could just fallback and render the current line as plaintext, if something in it caused a crash.

EDIT: Just gave it a try and found a very quick workaround that works fine though:

// Add it to the current inlines
try
{
    localContext.InlineCollection.Add(link);
    LinkRegister.RegisterNewHyperLink(link, element.Url);
}
catch
{
    // Invalid nested hyperlink, just add as a run
    link.Inlines.Clear();
    localContext.InlineCollection.Add(linkText);
}

This would be over this line (moving that link registering from the previous line as well): https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/a722f171cac7959d8fb39d732c5951018a2e8c07/Microsoft.Toolkit.Uwp.UI.Controls/MarkdownTextBlock/Render/MarkdownRenderer.Inlines.cs#L217

This way if the localContext.InlineCollection.Add(link); line fails, the duplicate link won't be registered, and the text will just be added as a simple run. But since this is already in a markdown link, the final result is exactly the expected text. Not really clean since the exception is actually being thrown, but it's something.

[michael-hawker]

@Sergio0694 thanks for the link!

Yeah, in many cases we should try and fail more gracefully and just render plaintext. @WilliamABradley any thoughts on a more general way to do this?

[windowstoolkitbot]

This issue seems inactive. It will automatically be closed in 14 days if there is no activity.