? Success
Pull Request for # 6149

User tests: Successful: Unsuccessful:

avatar MarkRS-UK
MarkRS-UK
11 Mar 2015

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.

Steps to reproduce the issue

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.

Expected result

The html <h1>Title</h1><p>Partial text</p>... is produced

Actual result

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.

System information (as much as possible)

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

Additional comments

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).

Votes

# of Users Experiencing Issue
1/1
Average Importance Score
4.00

avatar MarkRS-UK MarkRS-UK - open - 11 Mar 2015
avatar joomla-cms-bot joomla-cms-bot - change - 11 Mar 2015
Labels Added: ?
avatar zero-24 zero-24 - change - 11 Mar 2015
Rel_Number 0 6149
Relation Type Pull Request for
avatar Hackwar
Hackwar - comment - 11 Mar 2015

Would you be able to extend the existing unittest testcases regarding this with an example?

avatar zero-24 zero-24 - change - 12 Mar 2015
Category Libraries Unit Tests
avatar zero-24 zero-24 - change - 12 Mar 2015
The description was changed
avatar gogicomputers gogicomputers - test_item - 14 Mar 2015 - Tested successfully
avatar gogicomputers gogicomputers - test_item - 14 Mar 2015 - Not tested
avatar Bolzman
Bolzman - comment - 14 Mar 2015

Tested without applying the patch. Hm, could not see any truncation. Second was not produced.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6394.
avatar Bolzman
Bolzman - comment - 14 Mar 2015

edit ... Second < / h 1 > was not produced.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6394.
avatar FrankyDE
FrankyDE - comment - 14 Mar 2015

I can not reproduce the bug, therefore I can not test the patch :-(
My Text:
I entered in CodeMirror editor:

Title

Enough text


to get truncated
as the normal TinyMCE will change the code that it is not testable for this bug.
Maybe I misunderstand the bug, so maybe that is just because English is not my native language. In that case I am sorry.
avatar gorgonz
gorgonz - comment - 14 Mar 2015

can't reproduce it with TinyMCE. Added code with Editor and as 2. try i switched to raw code
=> effect not seen

avatar Harmageddon
Harmageddon - comment - 14 Mar 2015

@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.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6394.
avatar Harmageddon Harmageddon - test_item - 14 Mar 2015 - Tested successfully
avatar bertmert
bertmert - comment - 15 Mar 2015

@test Success

What means "unit tests"?

avatar zero-24 zero-24 - alter_testresult - 15 Mar 2015 - bertmert: Tested successfully
avatar Erftralle
Erftralle - comment - 15 Mar 2015

@test: bug confirmed, patch tested succesfully.

avatar Harmageddon Harmageddon - alter_testresult - 15 Mar 2015 - Erftralle: Tested successfully
avatar Harmageddon
Harmageddon - comment - 15 Mar 2015

@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

avatar gorgonz
gorgonz - comment - 16 Mar 2015

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


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6394.
avatar gorgonz gorgonz - test_item - 16 Mar 2015 - Tested successfully
avatar wilsonge wilsonge - change - 16 Mar 2015
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2015-03-16 16:59:36
avatar wilsonge wilsonge - close - 16 Mar 2015
avatar wilsonge wilsonge - reference | - 16 Mar 15
avatar wilsonge wilsonge - merge - 16 Mar 2015
avatar wilsonge wilsonge - close - 16 Mar 2015

Add a Comment

Login with GitHub to post a comment