[TYPO3-core] RFC: #10853: [Feature] New Multimedia CE

[Benjamin Mack]

Hey,

Here is my review for v17 (dude, 17 versions? ;-)).

===== Linespaces =====

As Franz Koch mentioned, there are still some whitespace changes.

-$incFile = $GLOBALS['TSFE']->tmpl->getFileName($filename);
-if ($incFile)	{
-	$fileinfo = t3lib_div::split_fileref($incFile);
+$incFile = $GLOBALS['TSFE']->tmpl->getFileName($filename);
+if ($incFile)	{
+	$fileinfo = t3lib_div::split_fileref($incFile);


I think he's referrring to the added lines ADD linespaces AFTER the ; on 
this line.


===== helper functions =====

I love "readFlexformIntoConf", this could be used for so many of my 
plugins. However, I don't like that it is merged into a parameter.

I would rather have a return value that is conf, and then we can merge 
the result with the $conf var. right?

   $ffConf = $this->flexform2array($flexData);
   $conf = array_merge_recursive($ffConf, $conf);


+	 * @param    boolean		is set if called recursive. Don't call function 
with this parameter, it's used inside the function only
+	 */
+	function readFlexformIntoConf($flexData, &$conf, $recursive=0) {
+		if ($recursive === 0) {

Well, the parameter $recursive is "0" by default, although it's boolean. 
Then you have a $recursive === 0 (which is a type comparison as well), 
so this is either wrongly coded or wrongly documented :)

What about "arrayKeyMap"? This should not stay in tslib_content as it's 
very generic. I'd also rename it, as array_map() is something else :). 
"remapArrayKeys" or something like that, in t3lib_div.


====== TS constants =======

+    # cat=content/cMedia; type=int+; label= Video Media Width: define 
the default width for the media-object	
+  videoPlayer = typo3/contrib/flashmedia/flvplayer.swf

+    # cat=content/cMedia; type=int+; label= Audio Media Width: define 
the default width for the media-object
+  audioPlayer = typo3/contrib/flashmedia/player.swf


Define the default width of the player?

====== MediaItems Hook ======

+if (is_array 
($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['tslib/hooks/class.tx_cms_mediaitems.php']['customMediaRender'])) 
{
+	foreach 
($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['tslib/hooks/class.tx_cms_mediaitems.php']['customMediaRender'] 
as $classRef) {
+		$hookObj = &t3lib_div::getUserObj($classRef);
+		$conf['file'] = $url;
+		$content = $hookObj->customMediaRender($renderType, $conf);
+	}
+}

Please add the $mode to the $conf variable as well.


==============

Apart from that, the latest patch (v17) does not apply anymore to 
current trunk!


All the best,
Benni.