Fix resolving of absolute URLs

[noerw]

URL resolver treats all URLs as path-relative, even path-absolute ones like / 🤔

The thing is, that resolveRelativeURL() is not only used for relative URLs.
This means, a link with a path-absolute URL like /foo/bar.jpg would be resolved as relative.

I'm not sure what the check for a leading / was meant for, this was in the repo from the first commit on (in a different variant having the same issue)..

[noerw]

ping

[muesli]

Sorry, for the late response!

We use this because you often encounter relative URLs in READMEs that point to an image or another document online. In such cases /foo/bar.jpg needs to be expanded to https://domain/foo/bar/.jpg. Maybe we could use path.Clean here instead?

[noerw]

We use this because you often encounter relative URLs in READMEs that point to an image or another document online. In such cases /foo/bar.jpg needs to be expanded to https://domain/foo/bar/.jpg.

@muesli But this doesn't work properly:

// This is the current behaviour:
resolveURL("https://example.com/a/b", "/foo/bar") == "https://example.com/a/b/foo/bar"
// Semantically correct & proposed here:
resolveURL("https://example.com/a/b", "/foo/bar") == "https://example.com/foo/bar"

Maybe we could use path.Clean here instead?

Doesn't seem like the right fit, relative things like ../ should not be dropped.

[6543]

I think there have to be Unit tests :D

[muesli]

@6543 You're right, we should add a few tests for this function.

@noerw I wonder if this isn't the correct behavior, though? The root path refers to the document's root, not the site's root.

Consider the following situations, which your PR would break:

resolveURL("https://github.com/a/b", "/foo/bar") == "https://github.com/a/b/foo/bar"
resolveURL("/home/user/Documents", "/foo/bar") == "/home/user/Documents/foo/bar"

[6543]

@muesli some background info... it's a dependency of https://gitea.com/gitea/tea/pulls/332 (currently fixed by replace :/ )

[muesli]

@6543 @noerw Nice, excited to see this in (gi)tea! Big fan!

[noerw]

resolveURL("/home/user/Documents", "/foo/bar") == "/home/user/Documents/foo/bar"

We might need to differentiate between URIs with & without host..

I think there have to be Unit tests :D

Will add some tests later

---

some github.com markdown testing (gitea equivalent)

• blob/master/ansi/renderer_test.go
• ansi/renderer_test.go
• /blob/master/ansi/renderer_test.go
• /ansi/renderer_test.go

[noerw]

Ok, treating subpaths as belonging to the base-namespace, as it would seem useful for github et al, is a special case deviating from the URI spec. As this lib is not specific to git-forge content, following the spec seems appropriate.
I'd argue that instead tools like tea and gh should go and set the base url as they need to get the wanted behaviour. something like

base := strings.TrimRight(repoURL, "/") + "/" # ensure trailing slash
href = strings.TrimLeft(href, "/") # ensure relative path

Looking at how github and gitea webfrontends (which we can treat as reference implementation for this usecase) render such URLs in markdown, it shows gitea handles repo path specially, and github plain follows URL spec

So i'd say this PR actually implements the right thing ;)

[muesli]

This still seems to break resolving URLs with various markdown links on GitHub. I'll add an example to the tests, so we can work on the implemenation from there.

[noerw]

I won't follow up on this; in tea we resolved this by preprocessing our links, but I can't remember the exact details and reasoning now. I'll close this, as glamour now works for us and our rather specialised use case wrt to relative URLs.
Sorry for all the confusion here 🙃