Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

string.truncate fails to match closing tags with numerals #6149

Closed
MarkRS-UK opened this issue Feb 22, 2015 · 5 comments · Fixed by #6394
Closed

string.truncate fails to match closing tags with numerals #6149

MarkRS-UK opened this issue Feb 22, 2015 · 5 comments · Fixed by #6394

Comments

@MarkRS-UK
Copy link
Contributor

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

@MarkRS-UK
Copy link
Contributor Author

Text fixed on gitHub, but that's not reflected here.
Click the black cat icon at the top of the Joomla issue tracker page to see the gitHub page that looks much better.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6149.

@Erftralle
Copy link
Contributor

Confirmed.

To reproduce the problem I created a new menu entry to show a list of my archived articles and configured it to show the intro text with a limit of 20 chars. This of course assumes that archived articles are available having an intro text containing the sample text mentioned above.

I think when searching for patterns like h1 in the opening tags they should be searched in the closing tags as well.
I confirm, that the patterns suggested by @MarkRS-UK are working fine for me.
I would prefer the second one (#</([a-z][a-z0-9]*)\b(?:[^>]*?)>#iU), as it seems to be more robust.

@MarkRS-UK, you should open a pull request for this issue.

@zero-24
Copy link
Contributor

zero-24 commented Feb 28, 2015

@MarkRS-UK

Text fixed on gitHub, but that's not reflected here.

you need to use: http://issues.joomla.org/tracker/joomla-cms/6149/edit else the tracker will sync back the changes 😄

As you have code you can easy submit a pull request. If you don't know how see here: https://docs.joomla.org/Using_the_Github_UI_to_Make_Pull_Requests

Thanks!


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6149.

MarkRS-UK pushed a commit to MarkRS-UK/joomla-cms that referenced this issue Mar 11, 2015
String truncate doesn't correctly match closing html tags that contain numbers.
This change corrects this.
Fixes joomla#6149
@joomla-cms-bot
Copy link

Set to "closed" on behalf of @zero-24 by The JTracker Application at issues.joomla.org/joomla-cms/6149

@zero-24
Copy link
Contributor

zero-24 commented Mar 12, 2015

Closing as we have a pull request here: http://issues.joomla.org/tracker/joomla-cms/6394 Thanks!


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6149.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants