Users lister "edit" links should be relative to site root and not the current URL

[Toutouwai]

Short description of the issue

The "edit" links for the Users lister look like this:

But links that are relative to the current URL like this are quite fragile as it will create an incorrect link if there are any URL segments (e.g. page numbers) with a trailing slash, yet URL segments are supposed to be equally supported with or without trailing slashes.

So a page URL like "/processwire/access/users/page2" will work but if the page URL is "/processwire/access/users/page2/" then the resulting edit link will be "/processwire/access/users/page2/edit/?id=1327" which fails:

It's more robust to make links relative to the site root, as is the case for the equivalent "edit" link in ProcessPageList:

Setup/Environment

• ProcessWire version: 3.0.222

[ryancramerdesign]

@Toutouwai Where is the /processwire/access/users/page2/ URL coming from? I usually prefer shorter/relative URLs where it makes no difference, but if there are URLs produced by ProcessUser or ProcessWire that get broken due to ProcessWire's output then I agree the absolute URL would be preferable and necessary. But before making that change, I'd like to know how to reproduce the issue.

[Toutouwai]

@ryancramerdesign The lister itself use URLs like /page2 - as can be seen by hovering the pagination links, or by dumping $input->url(true) after the lister executes.

The lister initially has these URLs without the trailing slash, but it doesn't do this consistently throughout the AJAX reloads that occur in the lister. And the trailing slash shouldn't result in different behaviour in any case as URL segments aren't supposed to care about trailing slashes one way or the other.

So with this hook in ready.php...

$wire->addHookAfter('ProcessUser::execute', function(HookEvent $event) {
    bd($event->wire()->input->url(true), "url");
});

...you sometimes get this on page 2...

...but then you reload the page and you get this...

In my module I want to be able to redirect to these URLs that the lister itself is producing without having broken edit links when the trailing slash is present.

[ryancramerdesign]

The lister itself use URLs like /page2

Unless I'm forgetting something, Lister uses them for ajax requests only. So the output is for use by the parent request, which doesn't use /page2 URLs, or at least it's not supposed to (though just tried it manually and it does seem to work). What I'm more curious about is if there's any way to reproduce the error just by clicking around in the admin, using it as intended, rather than modifying the address bar directly.

And the trailing slash shouldn't result in different behaviour in any case as URL segments aren't supposed to care about trailing slashes one way or the other.

URL segments may not care, but the application that uses them does. Some parts of the admin depend on trailing or non-trailing slash for URL segments, as there are instances where it's either not practical or too time consuming to determine the full URL for links. For those cases, href values like "./segment/" or "./segment" are used as an optimization. But only in cases where the URLs are those that are unlikely for someone to ever need to type in manually.

In the case of ProcessUser, the URLs it uses are generated by ProcessPageType, which also does the same for ProcessRole, ProcessPermission, ProcessLanguage, and potentially others. ProcessPageType uses relative URLs for things like "./add/" and "./edit/" since those are the segments it supports with executeSegment() methods. By using relative URLs it will automatically work with any module that extends it, no matter where it lives. ProcessPageType doesn't support pagination, that's just something that Lister uses on top of it during ajax requests.

We could support absolute URLs here, but it's not as simple as it sounds, and the admin does care about slash usage in more places than this, so I'm just trying to determine if the benefits in this case are greater than the costs.

[Toutouwai]

What I'm more curious about is if there's any way to reproduce the error just by clicking around in the admin, using it as intended, rather than modifying the address bar directly.

I suppose it depends on if the ability to develop custom modules that extend the functionality of core modules is part of what's intended in ProcessWire. In my case my module needs to check for a form submission from an admin page that's executing Lister, save some data from the submission, and then redirect back to the same URL that has just been executing the Lister. I don't manually add any trailing slash in my code - I just get the URL from $input->url() so I thought it should work.

In the case of ProcessUser, the URLs it uses are generated by ProcessPageType, which also does the same for ProcessRole, ProcessPermission, ProcessLanguage, and potentially others. ProcessPageType uses relative URLs for things like "./add/" and "./edit/" since those are the segments it supports with executeSegment() methods. By using relative URLs it will automatically work with any module that extends it, no matter where it lives. ProcessPageType doesn't support pagination, that's just something that Lister uses on top of it during ajax requests.

Not sure I understand this. My proposal is that links should include the URL of the page that is executing the Process module. So links would be specified with something like:

$this->wire()->page->url . 'edit/'

So if the concern is that there can be modules that extend ProcessPageType that might have methods executing at segments that ProcessPageType can't know about (e.g. I could have a module extending ProcessPageType that does something at /my-page/some-segment/some-other-segment/) then my proposal makes it more tolerant of such things. Because the edit method is something that executes specifically at the first URL segment and won't necessarily work relative to any URL that the Process module might make use of.

I tried replacing the relative links to the executeSomething() methods in ProcessPageType with links that include the Process page URL.

// 'editURL' => './edit/',
// 'addURL' => './add/',
'editURL' => $this->wire()->page->url . 'edit/',
'addURL' => $this->wire()->page->url . 'add/',

// ...

// $form->attr('action', './list');
$form->attr('action', $this->wire()->page->url . 'list');

Users, Roles and Permissions all seemed to work normally afterwards. Is there a downside to doing this?