[TYPO3-core] RFC: #10327: [felogin] redirecting to referer does not work
Steffen Kamper
info at sk-typo3.de
Mon Dec 7 09:21:33 CET 2009
Hi Benni,
Benjamin Mack schrieb:
> Hi Steffen,
>
> I reviewed your v4 version of the patch.
>
> First of all, your patch does not apply cleanly anymore for the 4_3
> branch. After taking care of that, redirecting in 4.3 finally works
> again, so this is a must have for 4.3! So: +1 after testing.
>
great! Many thanks for review and test!
> I also think that you might want to commit to 4.2, 4.3 and trunk.
> However, I recommend only to commit to 4.3 and trunk for now.
>
> Some remarks after reading your patch:
>
> + $return = implode(',', $redirect_url);
> + return t3lib_div::trimExplode(',', $return, TRUE);
>
> I would never use a keyword as variable name, it just confuses people.
> Just put everything in one line, that should work.
>
i wanted to mak it more easy to read, but this is not such complicate, i
can do it in one line.
> ==========
>
> Although this change is fine for trunk, please don't do that for a
> stable version, I recall one incident not too long ago where a change
> like this made some exts not to work anymore.
>
> - var $userIsLoggedIn; // Is user logged in?
> - var $template;
> - var $uploadDir;
> - var $redirectUrl;
> +
> + protected $userIsLoggedIn; // Is user logged in?
> + protected $template;
> + protected $uploadDir;
> + protected $redirectUrl;
> protected $noRedirect = false;
> + protected $logintype;
>
> For trunk: Could you also be more descriptive on the variables (what
> they do?).
>
yes, i remember that too. But never for a pi1. I also never seen
extension xclassing other pi-class, but maybe you did? Anyway these are
internal vars so they should be protected. I think for trunk should that
ok (not for 4.2)
I will take care for the commit to 4.3 and trunk today, thanks again.
vg Steffen
More information about the TYPO3-team-core
mailing list