WPCOM-Legacy-Redirector icon indicating copy to clipboard operation
WPCOM-Legacy-Redirector copied to clipboard

UI: Show absolute URLs as relative URLs

Open GaryJones opened this issue 6 years ago • 4 comments
trafficstars

If the start of the To URL matches the home value, then strip it off. This creates a more consistent view, and allows absolute URLs to stand out more. This can also be useful for spotting differences in protocol between the current site and the redirect destination.

Here's a screenshot from my local http system:

Screenshot 2019-05-27 at 14 25 33

Here's the same system with a https setup:

Screenshot 2019-05-27 at 14 27 11

Fixes #44.

GaryJones avatar May 27 '19 13:05 GaryJones

May want to standardize with wp_make_link_relative() instead of replacing all occurrences (or limit to first). @GaryJones

bdtech avatar May 27 '19 14:05 bdtech

May want to standardize with wp_make_link_relative() instead of replacing all occurrences (or limit to first). @GaryJones

Good to know, but the description says:

Removes the http or https protocols and the domain. Keeps the path '/' at the beginning, so it isn't a true relative link, but from the web root base.

Looking at the code shows it is a regex that replaces the protocol and domain - it doesn't take into account if it's a subsite in a multisite that has a base url of http://example.com/foo/.

GaryJones avatar May 27 '19 20:05 GaryJones

So with the proposed implementation you could be also replacing URLs in query string values. So at the very least good idea to limit it to one replacement. Not super common but I’ve seen URL’s used in UTM’s.

Exactly, it will make the URL relative. Which in this case will work as intended since this would be used instead of str_replace(). Since you’re checking home_url() is present already in the if condition, the subsite scenario would be accounted for as REQUEST_URI would be fully intact.

By replacing the entire home URL (which can include a subsite parent directory such as /subsite1/) it would actually be more confusing. Since the link displayed would be relative to the directory and not the domain (by definition a relative URL). https://example.com/subsite1/link/ would become /link/ which actually isn’t a valid relative URL.

bdtech avatar May 27 '19 20:05 bdtech

So with the proposed implementation you could be also replacing URLs in query string values. So at the very least good idea to limit it to one replacement.

I think this is a good point. I can't think of a scenario where a URL would it's own URL again in a query, but I'm sure someone somewhere is doing something like that for some reason. I can't see it hurting adding it in.

By replacing the entire home URL (which can include a subsite parent directory such as /subsite1/) it would actually be more confusing. Since the link displayed would be relative to the directory and not the domain (by definition a relative URL). https://example.com/subsite1/link/ would become /link/ which actually isn’t a valid relative URL.

This is a super interesting point. My initial thought would be that a sub site owner would view things relative to their "home" that they know and love, their subsite url example.com/subsite1/.

However after looking into this more and testing around, I think including the subsite path is important while showing/creating relative links so that no assumptions about whether or not to include it are incorrectly made. Ideally the submission process would handle either and notice the path was there or missing, but I feel for now including the subsites path is the best approach.

I checked out the leading redirection plugin in .org https://en-ca.wordpress.org/plugins/redirection/ and they do include the /subsite1/ path when they chop off to visually show a relative link:

Screen Shot 2019-06-04 at 3 39 51 PM

(while testing I stumbled on a few issues with relative paths in subsites & opened https://github.com/Automattic/WPCOM-Legacy-Redirector/issues/73)

davidsword avatar Jun 05 '19 17:06 davidsword

Thinking more, I think this needs a new approach. Closing this for now.

GaryJones avatar May 26 '24 18:05 GaryJones