[TYPO3-core] RFC #9615: Move frontend editing to a system extension

[Steffen Kamper]

Hi Feff,
Jeff Segars schrieb:
>> $hideField = 
>> $GLOBALS['TCA'][$table]['ctrl']['enablecolumns']['disabled'];
>>
>> before there was a check on $hideField, now there is no check. This 
>> cause a problem with
>> $recData[$table][$uid][$hideField] = 1;
>> when $GLOBALS['TCA'][$table]['ctrl']['enablecolumns']['disabled'] is 
>> NULL, is this filled all the time?
> 
> The code here is actually the same as before unless I'm overlooking 
> something.  Pasting it below...
> 
>> public function hide($table, $uid) {
>>     $hideField = 
>> $GLOBALS['TCA'][$table]['ctrl']['enablecolumns']['disabled'];
>>     if ($hideField) {
>>         $recData = array();       
>>         $recData[$table][$uid][$hideField] = 1;
>>         $this->tce->start($recData, array());
>>         $this->tce->process_datamap();
>>     }
>> }

this seems ok. Sometimes it looks different only looking to the diff 
where is saw this:
+		$hideField = 
$GLOBALS['TCA'][$table]['ctrl']['enablecolumns']['disabled'];
+			$recData[$table][$uid][$hideField] = 1;

but overseen the remaining if ($hideField) {
so it's ok.

> 
> 
>> strtolower($conf['allow']
>>
>> should be t3lib_div::strtolower($conf['allow'] for turkish reasons
> 
> In this case, allow is a list of actions that come from a predefined 
> list (show, edit, etc) so Turkish shouldn't really be a concern. I went 
> ahead and changed it just in case someone defines their own action though.
>

I was not sure where the actions are defined, as $conf normally is a 
user setting. Using EDIT here will break it. Calling t3lib_div-method 
doesn't cost anything, should also be faster as it makes a simple 
ascii-conversion.


>> There are some CGL-issues, the tabbed comments and some spaces missing 
>> after comma.
> 
> Scrubbed the file again for everything I saw.  Comments are actually 
> supposed to be tabbed until we have an official decision on that and an 
> update to the CGL, right?
>

i thought they start same position :-) but this isn't really important.

>  > class.tx_feedit_adminpanel.diff
>> ================================
>>
>> $row .= '<img src="' . TYPO3_mainDir . 'gfx/ol/' . ...
>>
>> please use scinning interface (and CGL)
> 
> I'm wondering if the skinning interface actually makes sense right now. 
> The call to setFancyDesign() ignores skinning so changing icons based on 
> the current skin could actually break things and at a minimum will cause 
> an inconsistent look and feel.
> 
> Not saying skinning support shouldn't be added, but its probably outside 
> the scope of this patch that is just for refactoring.
> 

sure, can be done in extra RFC.

>>  . chr(10)
>> just annotation - would be better to have global constant LINE_BREAK
> 
> If you're OK with it, I'll leave it as-is for now since the chr(10) is 
> used only 3 times in adminpanel and once in editpanel. If there was 
> already a global value, I could see using it but the usage is rare and I 
> don't see it as something that will be changing.
>

yes, that's why i said "annotation". Also something for an extra RFC to 
define a constant here.

> 
>> class.tx_feedit_editpanel.diff
>> ===============================
>>
>> +            $content = (strlen($cBuf) && $securCount) ? 
>> substr($content, 0, strlen($cBuf)) . $icon . substr($content, 
>> strlen($cBuf)) : $content = $icon . $content;
>>
>> this looks like a problem in utf8-mode with the native strlen, substr 
>> etc.
> 
> Are you OK with addressing this in a separate RFC? From a quick glance, 
> it looks like some brittle code to me and I'd rather give any changes 
> the full attention of a normal RFC rather than rolling it into a big 
> patch where it gets lost.
>

sure, can also be done in extra RFC. I only commented everything i saw 
while reviewing.

> 
> Thanks for the review and testing!
> Jeff
> 

thanks for the great patch!

vg Steffen