Skip to content

fix: resolve relative links and images against the article's URL - #202

Open
thebanri wants to merge 1 commit into
guyfedwards:masterfrom
thebanri:fix-relative-urls
Open

thebanri wants to merge 1 commit into
guyfedwards:masterfrom
thebanri:fix-relative-urls

Conversation

@thebanri

@thebanri thebanri commented Oct 4, 2026

Copy link
Copy Markdown

Following up on #201: this is one of the fixes from it, on Bubble Tea.

Feeds often use relative links and image sources (/doc/, next/, ../). nom converted them with an empty domain, so they stayed relative. They were neither clickable nor useful to copy. This resolves them against the article's own link with url.ResolveReference, the way a browser would.

The converter's built-in domain option wouldn't do it. It only sets the host, forces http, and gets path-relative URLs (next/, ../) wrong. Absolute URLs, mailto: and data: are left as they are. If the article has no absolute link, nothing changes.

Before → after, for an article at http://127.0.0.1:7781/blog/g1/:

See the docs /doc/ and next /next/.
See the docs http://127.0.0.1:7781/doc/ and next http://127.0.0.1:7781/blog/g1/next/.

goquery moves from indirect to direct in go.mod, because the option's function type names it. It was already in the module graph.

Tested with a table test (root-relative, path-relative, ../, fragment, //host image, relative image, absolute, mailto), which fails on master. Also checked by hand: built nom, ran it in a pty against a local feed, and read the article screen.

Copilot AI balanced review requested due to automatic review settings October 4, 2026 22:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.

2 participants