where to find the return Field of Step 3?

@franz-wohlkoenig inspect element and search for aW5kZXgucGhwP29wdGlvbj1jb21fYmFubmVycw I guess

I have tested this item ✅ successfully on f3d3fc5

Patch works as expected. Before patch applied I will not return to given page (com_banners) and after the patch I do return to the given page.

@franz-wohlkoenig @hans2103 is correct. You can do Inspect Element from your browser to find the field. I update the instructions accordingly.

thanks for Hints. Cant find in DevTools aW5kZXgucGhwP29wdGlvbj1jb21fYmFubmVycw or <input name="task" or <textarea like shown in Screenshot.

@franz-wohlkoenig Here are some more screenshots, hope that helps.

Before:

After:

I guess you should scroll down to almost the end of the document to see this.

Honestly I doubt this is a good idea.
If I remember right, we do some checks on redirect URLs to make sure you can't be redirected to an external URL this way. Someone from JSST should at least have a look at it to make sure we don't introduce issues. @SniperSister

at a minimum i would think we would need to ensure that the redirect is an internal url

@brianteeman Good point about the internal check. I have added the check to make sure the redirect URL is internal.

@Bakual The same functionality exists in the frontend, so I fail to see how this is worse than having it in the frontend.

@roland-d thanks for helping, but no Luck.

@franz-wohlkoenig What if you just look at the source of the page? You can right click the page and select View Page Source. On that tab that opens you can search for name="return". Does that work for you?

The same functionality exists in the frontend, so I fail to see how this is worse than having it in the frontend.

If you check and only allow internal URLs, then it should be fine I guess. But then I don't understand your comment, this is a library controller class. That one is used both in frontend and backend.

The only place I found where we currently use that code is \ContentControllerArticle::getReturnPage and there we check if it is internal as well.

If we add this to JControllerForm, shouldn't the frontend ContentControllerArticle class then be adjusted? The current code seems to be redundant then to me as it does the same the parent class already will do after this PR?

I have tested this item ✅ successfully on 589d565

@roland-d Source Code works. Tooks Time to understand where to look.

@Bakual

If we add this to JControllerForm, shouldn't the frontend ContentControllerArticle class then be adjusted?

I don't see why because it routes differently. The getReturnPage() has a hardcoded redirect to JUri::base() if no return is set. This is not what you want in the backend if you ask me.

The current code seems to be redundant then to me as it does the same the parent class already will do after this PR?

This method already calls the parent but redirects in other ways than the parent does.

The only thing I see that is weird is this piece of code which is unrelated to my change:

$return = base64_encode('index.php?Itemid=' . $menuitem . $lang);

			// If ok, redirect to the return page.
			if ($result)
			{
				$this->setRedirect(JRoute::_(base64_decode($return)));
			}

Why are we even encoding? ?

I don't see why because it routes differently. The getReturnPage() has a hardcoded redirect to JUri::base() if no return is set. This is not what you want in the backend if you ask me.

Ah true. Makes sense.

Why are we even encoding?

I gave up trying to understand all our code after I saw com_tags ?

PR ready for RTC?

Fine from me.

2 new Tests, @Bakual?

There was only one test after he added the internal check, and there was another commit after that one as well (allthought that one is safe for sure). So by our rules we would need at least one more test.

@hans2103 can you please retest?

I have tested this item ✅ successfully on 752415e

I can confirm the I get the actual result without the patch and the expected result with the patch.

RTC after two successful tests.

I have tested this succesfully.

I have tested this item ✅ successfully on 752415e

I can confirm that the return argument is not included, while it is included when the patch has been applied.