[TYPO3-core] RFC: #15576: Enable customization for the backend and frontend fatal error messages

[Reinhard Führicht]

Hi Steffen,

thank for your feedback.
See my comments below.

Am 27.08.2010 19:25, schrieb Steffen Gebert:
> Hi Reinhard,
>
> thanks for your patch. In principle a big "YES, finally! (somebody
> cared)" ;-)
>
> A few things, which prevent a positive review:
>
> * 'errorMessageLogo' => TYPO3_maindir . 'gfx/typo3logo.gif'
> - has to be TYPO3_mainDir

It worked that way for some reason, but of course, you are right.

> - -1 for a config option for this, IMHO the option to defina a custom
> template should be sufficient

OK for me.

>
> * changing signature of
> function·debug_typo3PrintError($header,·$text,·$js,·$baseUrl·=·'')
> You're removing a feature - to print the error message in an alert
> box.

I did a search and the last 2 arguments are used nowhere, so I thought 
maybe some old code used them and they are obsolete now. But if they 
should stay in, no problem.

>
> * naming of typo3PrintError()
> I'm strongly against reusing this name! Maybe printError() or similar.

OK.

>
> * the most important point, while we're at changing this functionality:
> Long-term plan is to throw exceptions everywhere and get rid of
> typo3PrintError(), isn't it?
> IIRC the production exception/error handler calls typo3PrintError().
> What do you think of modifiying this error handler and implement the
> functionality there? While deprecation phase, t3PE() can throw an
> exception, until it's removed.

Yes, modifying the error handler would be a good solution. I will have a 
look.

>
> * CGL and nitchpicking:
> - Please uses spaces after "if" before the (
> - only one space after a comment (not // prepend "/")
> - no tabs in "empty" lines
> so maybe you could enable your IDE to show white-space chars while
> you're at core developement? ;-)
> - use uppercase for boolean values TRUE/FALSE
> - $logo·=·'<img·src="'·.·$imageSrc·.·'"·/>';
> where's the alt="Logo" or similar ;-)

You are right, I should care more about that. But I tend to propose a 
version 1 on the list, wait for feedback and care about CGL when the 
logic of the patch is approved.
Thanks for the "show white-space chars" hint, that coud help indeed. :-)

> - print·$errorMessage;
> why print? we use echo everywhere. For the sake of consistency I'd
> keep echo. Or is there any advantage?

I don't know about any advantage. Let's just change it back to echo.

>
> Attached is a cleaned patch (CGL and so on changed), to continue working.
>
> Thanks for your efforts, Reinhard!
>
> Kind regards
> Steffen
>

Thanks again for having a look at the patch.

Kind regards,
Reinhard