User tests: Successful: Unsuccessful:
String truncate doesn't correctly match closing html tags that contain numbers.
This change corrects this.
Fixes #6149
Truncated text from articles containing an html element with numerals (currently only "h"(eader) exists) and another html element whose closing tag is removed in the truncation, will produce invalid html.
This is because the regex in string.truncate fails to match closing tags with numerals.
Put text like <h1>Title</h1><p>Enough text to get truncated</p>
at the beginning of an article that is then displayed with string.truncate.
The html <h1>Title</h1><p>Partial text</p>...
is produced
The html <h1>Title</h1><p>Partial text</p></h1>...
is produced.
This is invalid in that it produces a second, unmatched, closing h1 tag.
The incorrect code is in line 100 in /libraries/cms/html/string.php.
The match string is #</([a-z]+)>#iU
when it should be #</([a-z][a-z0-9]*)>#iU
to match closing tags with numerals.
This renders correct html in the above example.
If we wanted to cover the case where a closing tag like </h1 >
was being matched, the pattern should be #</([a-z][a-z0-9]*)\b(?:[^>]*?)>#iU
This error, of course, afflicts truncateComplex too, since it calls truncate.
This code is called anywhere where a short extract from html is displayed, six different views in core components (content, tags, finder, newsfeed).
Labels |
Added:
?
|
Rel_Number | 0 | ⇒ | 6149 |
Relation Type | ⇒ | Pull Request for |
Category | ⇒ | Libraries Unit Tests |
Tested without applying the patch. Hm, could not see any truncation. Second was not produced.
edit ... Second < / h 1 > was not produced.
I can not reproduce the bug, therefore I can not test the patch :-(
My Text:
I entered in CodeMirror editor:
Enough text
can't reproduce it with TinyMCE. Added code with Editor and as 2. try i switched to raw code
=> effect not seen
@FrankyDE @gorgonz
Try to insert the following in a .php file, e.g. the index.php of your template:
<?php echo JHtmlString::truncate("<h1>Header</h1> Lorem ipsum dolor sit amet blind text", 20); ?>
@test:
Patch solves the reported bug. Only missing thing would be a unit test as proposed by @Hackwar, but the patch itself works fine.
@bertmert Unit tests are PHP programs that test other PHP functions to make sure that they produce the desired result. So for this one, we could need a test that makes sure that the truncate method returns correct results for a few different inputs.
See also: https://docs.joomla.org/Unit_Testing and https://docs.joomla.org/Category:Automated_Testing
works!
added the mentioned code to the isis index.php
before:
=> wrong tag is there
after patch install
=> correct functionality of truncate()
undo patch
=> error is here again
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-03-16 16:59:36 |
Would you be able to extend the existing unittest testcases regarding this with an example?