[#23242] - com_content Ensure introtext and fulltext are valid html
- Closed
- 10 Dec 2018
- Medium
- Build: staging
- # 23242
- Diff
- Chrissi2812:fix/introtext_html
User tests: Successful: Unsuccessful:
We had issues with the readmore tag causing the webpage layout to collaps due to unclosed <div>
Summary of Changes
The content for introtext and fulltext are parsed by DOMDocument.
So no longer invalid html gets saved to database.
Testing Instructions
Try to save following article with a Readmore tag.
<div>This is a Simple Text</div>
<div><hr id="system-readmore" /></div>
<div>The Fulltext</div>Expected result
The Intro should be:
<div>This is a Simple Text</div>Or
<div>This is a Simple Text</div>
<div></div>Actual result
<div>This is a Simple Text</div>
<div>This breaks the article listing.
Documentation Changes Required
None.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries External Library Composer Change |
| Labels |
Added:
?
?
|
||
Thanks for your advice with the plugin 👍🏽
I have put together a simple plugin
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-12-10 07:40:06 |
| Closed_By | ⇒ | Chrissi2812 |
Some 3rd party editors are already trying to do this at browser (client) side, to close and reopen at least 1 parent HTML tag (or all the nested of tags)
you were testing with tinymce ?
If this does not work with tinymce (i have not tested yet) , then this is a real bug
I do not say to make a fix or not at client side
but at least a fix in the Table or in the Model should be made
@ggppdk Yes I have used tinymce.
It only happens when the content contains a <div>.
We didn't encountered it our self (a client found this) so we are not 100% sure were the content got pasted from.
To replicate you have to use this in Source Edit mode:
<div></div>After this all line breaks are new <div> and cause the <hr> to be wrapped by these.
So is this a bug of joomla or tinymce?
It is definitely not a tinyMce bug, as introtext and fulltext are not related to tinyMce code ...
i just mentioned the fact that some 3rd party editors choose to try to make a fix at client side
and also a possibility is to make fix at client side, but even in this case it is not tinyMce bug
About being a bug or not
since we cannot guarantee where the user will place the readmore,
i would say it is an issue with any core or 3rd party extension
that does not make a check if readmore has broken the HTML in bad place
i would say it is an issue with any core or 3rd party extension
that does not make a check if readmore has broken the HTML in bad place
Even if it is a bug the approach in this PR has too many potential issues to be included in core.
DOMDocumentis known to not be HTML5 friendly- The hook modifying the data is too close to the database (this is the type of validation that belongs in a model with a parameter and potential alert to the user, not in a table (ORM entity) with no parameter and no alert to the user their markup may have changed (and if the readmore split is actually done in the table I'd say that's wrong too, that should be a model doing it)
- If anything, the check should at most ensure that when the content is split that whatever element is open before the readmore tag is closed (and only that element) on the introtext side, similar for the fulltext side; we shouldn't be parsing/processing the entire text field
TinyMce 4 has "CMS support" with a pagebreak plugin that will close parent tags it also accepts custom text as break
https://www.tiny.cloud/docs/plugins/pagebreak/
maybe we can make use of it for Joomla readmore and pagebreak buttons ?
We need a solution that's either part of Joomla's Editor JavaScript API or a server-side solution. Relying on editor specific plugins isn't portable; the same problem would just crop up if you don't use the core TinyMCE option.
This type of behavior honestly belongs in a model (or potentially a plugin, but definitely not the table class), and presumably behind some kind of option. This is one of those cases where you're changing user input without any type of communication and even though it has the best of intentions we still shouldn't be doing that. Also, be aware that
DOMDocumentlacks HTML5 support and can potentially wreak havoc on your content.As far as the
DomHelperclass goes, new files shouldn't be added directly to thelibraries/vendordirectory in the CMS as these all come from external packages/repositories. This class should be submitted as a pull request to https://github.com/joomla-framework/utilities instead to be added to the same spot.