Skip to content

Fix Markdown links event bug #6093

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Fix Markdown links event bug #6093

wants to merge 5 commits into from

Conversation

Aearsears
Copy link

@Aearsears Aearsears commented Aug 31, 2025 •
edited
Loading

Relates to #6039

Hi all first time contributor around here. I had a look into this bug where clicking on a MD link would cause FileNotFoundError. What seemed to be happening is that MarkdownViewer and its child Markdown both handled the Markdown.LinkClicked message. I thought the order of messages isnt guaranteed but the child Markdown would get the message first?

To fix this, if its a HTTP link I let Markdown handle it and stop propagation to parent. If its an anchor or a file, MarkdownViewer handles it. please let me know your review

Please review the following checklist.

  • Docstrings on all new or modified functions / classes
  • Updated documentation
  • Updated CHANGELOG.md (where appropriate)

@Aearsears Aearsears force-pushed the fix-md-links-event-bug branch from 1fc8654 to 2b00b7b Compare September 3, 2025 20:14
@Aearsears Aearsears force-pushed the fix-md-links-event-bug branch from d3d9485 to 9ccc785 Compare September 3, 2025 20:19
@Aearsears Aearsears marked this pull request as ready for review September 3, 2025 20:30
willmcgugan
willmcgugan requested changes Sep 28, 2025
View reviewed changes
Copy link
Member

@willmcgugan willmcgugan left a comment

Choose a reason for hiding this comment

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

Can we have tests to cover more code paths?

Suggest you implement an open_url in app to check if the expected urls where called.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@willmcgugan willmcgugan willmcgugan requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Aearsears @willmcgugan