Fix: dol_htmlentitiesbr behaviour was different if input was html

or not. Add phpunit test to avoid that in future.

htdocs/core/lib/functions.lib.php

@@ -3459,25 +3459,25 @@ function dol_nl2br($stringtoencode,$nl2brmode=0,$forxml=false)
  *	This function is called to encode a string into a HTML string but differs from htmlentities because
  * 	all entities but &,<,> are converted. This permits to encode special chars to entities with no double
  *  encoding for already encoded HTML strings.
- * 	This function also remove last CR/BR.
+ * 	This function also remove last EOL or BR if $removelasteolbr=1 (default).
  *  For PDF usage, you can show text by 2 ways:
  *              - writeHTMLCell -> param must be encoded into HTML.
  *              - MultiCell -> param must not be encoded into HTML.
  *              Because writeHTMLCell convert also \n into <br>, if function
- *              is used to build PDF, nl2brmode must be 1
+ *              is used to build PDF, nl2brmode must be 1.
  *
  *	@param	string	$stringtoencode		String to encode
  *	@param	int		$nl2brmode			0=Adding br before \n, 1=Replacing \n by br (for use with FPDF writeHTMLCell function for example)
  *  @param  string	$pagecodefrom       Pagecode stringtoencode is encoded
  *  @return	string						String encoded
  */
-function dol_htmlentitiesbr($stringtoencode,$nl2brmode=0,$pagecodefrom='UTF-8')
+function dol_htmlentitiesbr($stringtoencode,$nl2brmode=0,$pagecodefrom='UTF-8',$removelasteolbr=1)
-{
-	if (dol_textishtml($stringtoencode))
 {
 	$newstring=$stringtoencode;
+	if (dol_textishtml($stringtoencode))
+	{
 		$newstring=preg_replace('/<br(\s[\sa-zA-Z_="]*)?\/?>/i','<br>',$newstring);	// Replace "<br type="_moz" />" by "<br>". It's same and avoid pb with FPDF.
-		$newstring=preg_replace('/<br>$/i','',$newstring);	// Remove last <br>
+		if ($removelasteolbr) $newstring=preg_replace('/<br>$/i','',$newstring);	// Remove last <br> (remove only last one)
 		$newstring=strtr($newstring,array('&'=>'__and__','<'=>'__lt__','>'=>'__gt__','"'=>'__dquot__'));
 		$newstring=dol_htmlentities($newstring,ENT_COMPAT,$pagecodefrom);	// Make entity encoding
 		$newstring=strtr($newstring,array('__and__'=>'&','__lt__'=>'<','__gt__'=>'>','__dquot__'=>'"'));
@@ -3485,7 +3485,8 @@ function dol_htmlentitiesbr($stringtoencode,$nl2brmode=0,$pagecodefrom='UTF-8')
 	}
 	else
 	{
-		$newstring=dol_nl2br(dol_htmlentities($stringtoencode,ENT_COMPAT,$pagecodefrom),$nl2brmode);
+		if ($removelasteolbr) $newstring=preg_replace('/(\r\n|\r|\n)$/i','',$newstring);	// Remove last \n (may remove several)
+		$newstring=dol_nl2br(dol_htmlentities($newstring,ENT_COMPAT,$pagecodefrom),$nl2brmode);
 	}
 	// Other substitutions that htmlentities does not do
 	//$newstring=str_replace(chr(128),'&euro;',$newstring);	// 128 = 0x80. Not in html entity table.     // Seems useles with TCPDF. Make bug with UTF8 languages

test/phpunit/FunctionsTest.php

@@ -192,15 +192,30 @@ class FunctionsTest extends PHPUnit_Framework_TestCase
      */
     public function testDolHtmlCleanLastBr()
     {
+        $input="A string\n";
+        $after=dol_htmlcleanlastbr($input);
+        $this->assertEquals("A string",$after);
+
+        $input="A string first\nA string second\n";
+        $after=dol_htmlcleanlastbr($input);
+        $this->assertEquals("A string first\nA string second",$after);
+
+        $input="A string\n\n\n";
+        $after=dol_htmlcleanlastbr($input);
+        $this->assertEquals("A string",$after);
+
         $input="A string<br>";
         $after=dol_htmlcleanlastbr($input);
         $this->assertEquals("A string",$after);
+
         $input="A string first<br>\nA string second<br>";
         $after=dol_htmlcleanlastbr($input);
         $this->assertEquals("A string first<br>\nA string second",$after);
+
         $input="A string\n<br type=\"_moz\" />\n";
         $after=dol_htmlcleanlastbr($input);
         $this->assertEquals("A string",$after);
+
         $input="A string\n<br><br />\n\n";
         $after=dol_htmlcleanlastbr($input);
         $this->assertEquals("A string",$after);
@@ -215,22 +230,44 @@ class FunctionsTest extends PHPUnit_Framework_TestCase
      */
     public function testDolHtmlEntitiesBr()
     {
-        $input="A string\nwith a é, &, < and >.";   // Text not already HTML
+    	// Text not already HTML
+
+    	$input="A string\nwith a é, &, < and >.";
         $after=dol_htmlentitiesbr($input,0);    // Add <br> before \n
         $this->assertEquals("A string<br>\nwith a &eacute;, &amp;, &lt; and &gt;.",$after);
 
-        $input="A string\nwith a é, &, < and >.";   // Text not already HTML
+        $input="A string\nwith a é, &, < and >.";
         $after=dol_htmlentitiesbr($input,1);    // Replace \n with <br>
         $this->assertEquals("A string<br>with a &eacute;, &amp;, &lt; and &gt;.",$after);
 
-        $input="A string<br>\nwith a é, &, < and >.";   // Text already HTML, so &,<,> should not be converted
+        $input="A string\nwith a é, &, < and >.\n\n";	// With some \n at end that should be cleaned
+        $after=dol_htmlentitiesbr($input,0);    // Add <br> before \n
+        $this->assertEquals("A string<br>\nwith a &eacute;, &amp;, &lt; and &gt;.",$after);
+
+        $input="A string\nwith a é, &, < and >.\n\n";	// With some \n at end that should be cleaned
+        $after=dol_htmlentitiesbr($input,1);    // Replace \n with <br>
+        $this->assertEquals("A string<br>with a &eacute;, &amp;, &lt; and &gt;.",$after);
+
+        // Text already HTML, so &,<,> should not be converted
+
+        $input="A string<br>\nwith a é, &, < and >.";
         $after=dol_htmlentitiesbr($input);
         $this->assertEquals("A string<br>\nwith a &eacute;, &, < and >.",$after);
 
-        $input="<li>\nA string with a é, &, < and >.</li>\nAnother string";   // Text already HTML, so &,<,> should not be converted
+        $input="<li>\nA string with a é, &, < and >.</li>\nAnother string";
         $after=dol_htmlentitiesbr($input);
         $this->assertEquals("<li>\nA string with a &eacute;, &, < and >.</li>\nAnother string",$after);
 
+        $input="A string<br>\nwith a é, &, < and >.<br>";	// With some <br> at end that should be cleaned
+        $after=dol_htmlentitiesbr($input);
+        $this->assertEquals("A string<br>\nwith a &eacute;, &, < and >.",$after);
+
+        $input="<li>\nA string with a é, &, < and >.</li>\nAnother string<br>";	// With some <br> at end that should be cleaned
+        $after=dol_htmlentitiesbr($input);
+        $this->assertEquals("<li>\nA string with a &eacute;, &, < and >.</li>\nAnother string",$after);
+
+        // TODO Add test with param $removelasteolbr = 0
+
         return true;
     }