[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