[TYPO3-core] Reminder#3: RFC: Bug 1477 and 3013 - xhtml_cleaning corrupts javascript

Franz Holzinger franz at fholzinger.com
Mon Mar 3 13:15:24 CET 2008


Hello Michael,

thanks for having a look on this issue again.

> As far as I can see you have received plenty of feedbacks and better
> suggestions on how to solve this problem, but at the same time refused to
> even look at them! Instead you asked on February 16:
> 
> "What is wrong with my patch? What is missing? What should be changed?
>  It has already been sent to the Core list."

This is not true. I have already looked at your patch in July 2007 and
have even asked a question about it and you have told me to go ahead. I
have answered to your patch on 26.07.2007 on the Core list, but did not
get any answer yet. The post on Februar 16 is only a reminder.

But now, one year later, you tell me to do another thing.
My patch is alreay from December 6th 2006 and all comments have already
been implemented in it.
But IMHO your patch does not contain the CDATA addition. So I have asked
again.

> After having one more look at it, I would simply say that I just don't like
> the way you have solved this. Your changes apply directly inside the loop
> which could have many pitfalls.
> 
> Just have a look at the alternative patches made by me or Sebastian
> (attached to duplicate issue number #1292) and you will see what I mean,
> and this is definitely a much more safe attempt on solving this problem.

If you do not need a better speed, this is your decision.
I have tried to invoid to add additional loops.
Of course, each PHP code can have pitfalls.

> If you are really interested to solve this, you should either start to
> explain why your patch is so much better, or otherwise publish a new RFC
> using one of the alternative patches.

I wanted to improve my patch or rewrite it, if something is missing or
badly coded.
I did not want to take your patch, because I think it is much slower
because of using regular expressions and string replacement on the whole
string, which might get very long on big pages, inside of loops.
If you prefer easier code to faster speed, then it does not make sense
to me to discuss about it. IMHO a regular expression is also hard to
understand. However your code has a double loop. Each additional
parameter string to $preserveContent will mean an additional loop.

Sebastionan's patch is the most easy and understandable solution. It
does not have preg inside loops like your patch. But it only works it a
HTML comment is used inside of the JavaScript tag.

See also
http://bugs.typo3.org/view.php?id=1477

1) Sebastion (see attached lines)
2) Franz (class.t3lib_parsehtml.diff)
3) Michael (bug_1477_stucki.diff)

But the CDATA addition is still not implemented in 1 and 3.

So please add your patch here to get it commited.

- Franz

------------------------------------------------------------------------

diff -ur TYPO3core_orig_cvs/t3lib/class.t3lib_parsehtml.php
TYPO3core_patch1292/t3lib/class.t3lib_parsehtml.php
--- TYPO3core_orig_cvs/t3lib/class.t3lib_parsehtml.php	2005-10-21
10:06:58.000000000 +0200
+++ TYPO3core_patch1292/t3lib/class.t3lib_parsehtml.php	2005-11-08
22:20:58.000000000 +0100
@@ -599,6 +599,9 @@
 	 */
 	function HTMLcleaner($content,
$tags=array(),$keepAll=0,$hSC=0,$addConfig=array())	{
 		$newContent = array();
+			// protect HTML comments from getting cleaned
+		preg_replace('/<!--(.*?)-->/','###{\\1}###',$content);
+
 		$tokArr = explode('<',$content);
 		$newContent[] = $this->processContent(current($tokArr),$hSC,$addConfig);
 		next($tokArr);
@@ -783,7 +786,10 @@
 			}
 		}

-		return implode('',$newContent);
+			// replace markers with the according comments to preserve HTML
comments (see the beginning of this function for the corresponding code)
+		preg_replace('/###{(.*?)}###/','<!--\\1-->',$content);
+
+		return $newContent;
 	}

 	/**

---------------------------------------------




More information about the TYPO3-team-core mailing list