Add redirect module to core.

[quicksketch]

Fixes backdrop/backdrop-issues#905.

Replaces PR at #1277.

[jenlampton]

This is looking pretty good! One thought, I'd like to see the same #field_prefix on the FROM and TO fields when creating or editing a redirect

LMK if you'd like things like this in a follow-up?

[quicksketch]

One thought, I'd like to see the same #field_prefix on the FROM and TO fields when creating or editing a redirect

The reason the prefix isn't on the TO field is because you can create external redirects (as noted in the help text). This allows you to do a redirect from http://www.example.com/blog to http://blog.example.com, or other such redirects.

[jenlampton]

Hm. that feels weird.

[quicksketch]

Hm. that feels weird.

Well, maybe best for a followup then? Redirect has always had this ability, I know of sites that have used it heavily for marketing purposes, e.g. an email or TV ad that is example.com/SALE that redirects to somecrazyexamplesale.com that is too hard to remember, so they use the recognizable, short domain name to redirect to the one-off microsite for the event/ad.

[jenlampton]

I agree, a follow-up. It's not the ability that feels weird, it's the UI for it. these two things are not the same :)

[klonos]

No PR sandbox 😞

[klonos]

Well, maybe best for a followup then?

Done!

[backdrop-ci]

Website: http://1317.backdrop.backdrop.qa.backdropcms.org
Username: admin
Password: MQ3r6f7U

[backdrop-ci]

Website: http://1317.backdrop.backdrop.qa.backdropcms.org Removed

[backdrop-ci]

Website: http://1317.backdrop.backdrop.qa.backdropcms.org
Username: admin
Password: ocacRyDE

[backdrop-ci]

Website: http://1317.backdrop.backdrop.qa.backdropcms.org Removed

[backdrop-ci]

Website: http://1317.backdrop.backdrop.qa.backdropcms.org
Username: admin
Password: rBxKoEok

[docwilmot]

Looks great. A couple issues with validation:

I typed ))+@+_@!_@+)!#)+!# in the "To" field and that didn't error. It's been saved as a local path (ie as <sandbox>/))+@+_@!_@+)!#)+!#. The same thing errors if you try to add it as a menu item under admin/structure/menu/manage/main-menu/add so core definitely doesn't accept it as valid, yet redirect_element_validate_redirect() does.

The other query is that _redirect_extract_url_options() which is called by redirect_element_validate_redirect() seems familiar; doesn't some core function already do that somewhere? If so we should update that core function instead, or if not it seems like it might be a useful utility function; couldnt it be put in core somewhere (system_extract_url_options()?) for general use?

[docwilmot]

I think maybe I was recalling url() which is nothing close to this, so please ignore the second point.

[quicksketch]

I typed ))+@+@!@+)!#)+!# in the "To" field and that didn't error.

Even though it's unusual it's not impossible. We specifically allow # in the destination (but not source) URLs for fragments, and all other characters are allowed in URLs as well. As long as they are encoded it's all okay. I suppose the question might be should we restrict the "To" field to only existing paths? That's why you can't add a menu item to this URL (that plus the fragment # symbol, which would never be allowed in a menu item or redirect source). So if the path doesn't exist, you shouldn't be able to redirect to it? This would run counter to the feature request to remove this limitation from the menu items. See backdrop/backdrop-issues#422.

We could also throw a warning as a half-way compromise. This form already has some fancy validation on it to give the user a warning but allow saving anyway if they really want (e.g. when redirecting from a real path like node/1 to something else like node/2). It's not recommended but it lets you do it if you really want.

[quicksketch]

The other query is that _redirect_extract_url_options() which is called by redirect_element_validate_redirect() seems familiar; doesn't some core function already do that somewhere?

The redirect_parse_url() function is very similar in functionality to PHP's parse_url(). I'm not sure why Redirect module reimplemented that instead of using the PHP version. I'll look at the project history.

[quicksketch]

Redirect module reimplemented that instead of using the PHP version. I'll look at the project history.

Ah, it uses parse_url() internally. It just expands on the functionality to make it more Backdrop-friendly. I'll add additional documentation.

[docwilmot]

It is definitely strange to include the ability to redirect to a non-existent path, when Redirect module is created partly to fix the issue of non-existent paths (404s). I'd vote to prevent this, even if we decide to go ahead with backdrop/backdrop-issues#422.

But this is in no way a blocker, and we can always decide after backdrop/backdrop-issues#422. So RTBC for me.

[backdrop-ci]

Website: http://1317.backdrop.backdrop.qa.backdropcms.org Removed

[quicksketch]

Merged for 1.4.0. Yay!

[klonos]

I don't have immediate need to use this, but yay indeed! I realize there was a lot of effort that went into this, so thanx and well done everybody 👏