WIP catch an edge case for parsing #20
Conversation
At current any field with a Shortcode in it (e.g. [code] ) is translated as expected into it's replacement content, however thanks to this html5 module it also comes accompanied by a random </img> tag at the end.
NightJar
changed the title
NightJar
changed the title
|
that is so fantastically bizarre |
|
Sure is, but it also appears to be 100% to do with the way we 'handle' shortcodes. In that we don't really handle them, we just throw regex at it and hope for the best, then inject invalid tags so we can find the reference again later. Have an upcoming PR for that too (proper parsing). |
Upon outputting a void element, the html5 library in use previous to this commit would mysteriously append closing tags to void elements - i.e. <img /> would become <img></img>. Reading upon the internet at large, this seems to be a known issue with PHP's DOMDocument library, however I could not find a reason why the html5 library at it's standard would not recreate the issue, however it remained would within our framework (requiring only silverstripe/recipe-core as a baseline to test this). So this commit has found and employed the use of another library with the same goal - making html5 easy to process. It appears to succeed where the previous did not - the test previously added to trigger the described undesirable functionality is now passing - there are no longer any invalid closing tags present in HTMLValue output. Please note that while the version of the new html5 processing library in use is in fact an out of date one, it is the last version compatible with PHP 5.6 - which we still support at this time.
|
Turns out no, 100% to do with some weird environmental thing with the backing library. Expanded investigation lead to see that this will also solve #19 |
|
Nice work :-) |
| return 'bit of test shortcode output'; | ||
| } | ||
| ); | ||
| $content = DBHTMLText::create('Test', ['shortcodes'=>true]) |
There was a problem hiding this comment.
the => needs spaces around it.
There was a problem hiding this comment.
https://travis-ci.org/silverstripe/silverstripe-html5/jobs/355285299#L628 :P
Yeah, you're probably right :) #21
| } | ||
| ); | ||
| $content = DBHTMLText::create('Test', ['shortcodes'=>true]) | ||
| ->setValue('<p>Some content with a [test_shortcode] and a <br /> followed by an <hr> in it.</p>') |
There was a problem hiding this comment.
I think this test is testing 2 things (1. that the HTML parser moves <hr> out of its <p> and 2. that the shortcode is correctly parsed.
I don't think there's a need for the first test (the lib itself should be testing for this) but if you want to test it, it should be a separate test.
There was a problem hiding this comment.
No, you're right. It should be tested in the lib. And it shouldn't be an issue.
But neither should it feel the need to close void elements (nor do they in the lib tests).
The opportunity was there, I took it. It is valid in this test, and it is only testing that the <hr> doesn't render as <hr></hr>, I've simply left the comment to make clear why the tag order has rearranged in the expected output (for those who perhaps may not be aware).
There's no harm in double checking our facts where there's history of them being alternative ;)
|
@robbieaverill you're too keen, my man |
At current any field with a Shortcode in it (e.g. [code] ) is translated
as expected into it's replacement content, however thanks to this html5
module it also comes accompanied by a random tag at the end.