Skip to content

Fix wikifying of root-relative links#330

Merged
matthijskooijman merged 2 commits intomasterfrom
root-relative-links
Aug 10, 2020
Merged

Fix wikifying of root-relative links#330
matthijskooijman merged 2 commits intomasterfrom
root-relative-links

Conversation

@matthijskooijman
Copy link
Copy Markdown
Contributor

This makes sure that a link like /en/pagename is properly wikified too, rather than just en/pagename or https://domain.tld/en/pagename.

Copy link
Copy Markdown
Contributor

@laurensmartina laurensmartina left a comment

Choose a reason for hiding this comment

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

Please check my comment.

Comment thread system/core/base.php
global $O_O;

// Handle urls like https://mydomain.tld/path/en/pagename
$remove = $O_O->getRequest()->getRootUrl();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it could. However, this is already the case with the current code (that part is not really changed, just refactored to use HyphaRequest::getRootUrl rather than global $hyphaUrl, I just split this commit into two to emphasize this).

Also, this is only a problem when someone externally adds a URL to another version (another equivalent domain or http vs https) of the site, since all urls internally generated use the same url as is removed here. IOW, once the url is made relative once, it gets stored in the database relatively properly, and is potentially expanded to an absolute URL when e.g. editing the page, but can then always be made relative again when saving.

Also, we cannot really fix this properly now: The PHP code has no idea about the different domains that the site is available under. The only way AFAICS to fix this is to configure these domains manually, which makes this a lot more complex. So I would leave this problem unsolved for now, certainly within this PR>

@matthijskooijman
Copy link
Copy Markdown
Contributor Author

I rebased on top of master and split the last commit in two. The first commit has disappeared, since it was already merged in another PR. I would think this is ready to merge now.

Previously, this would use the `$hyphaUrl` global variable, which has
the same value, but going through HyphaRequest makes for cleaner code.
This makes sure that a link like `/en/pagename` is properly wikified
too, rather than just `en/pagename` or `https://domain.tld/en/pagename`.
@matthijskooijman matthijskooijman merged commit dfac219 into master Aug 10, 2020
@matthijskooijman matthijskooijman deleted the root-relative-links branch August 10, 2020 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants