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

HTML on one line renders incorrectly #44

Open
jasonvarga opened this issue Feb 5, 2015 · 15 comments · May be fixed by #110
Open

HTML on one line renders incorrectly #44

jasonvarga opened this issue Feb 5, 2015 · 15 comments · May be fixed by #110

Comments

@jasonvarga
Copy link

If you have HTML on a single line, only the first will render.

See this screenshot of the behavior from the demo.

Only happens in Parsedown Extra. Regular Parsedown is fine.

@jtojnar
Copy link

jtojnar commented Feb 17, 2015

Also occurs when there is nbsp (u+a0) at the end of the line.

<div></div> 

Google is eaten

It seems only block elements do this. Inline elems are unaffected. Only Parsedown Extra is affected.

@jaswrks
Copy link

jaswrks commented Mar 15, 2015

I can confirm that this is a serious issue. What makes this a particularly nasty bug is that it results in a complete loss of the data as it was originally intended.

2015-03-14_19-04-00

<div>1</div><p>2</p>
The p tag (and contents), along with this line are eaten.

@jaswrks
Copy link

jaswrks commented Mar 15, 2015

I left a small donation hoping that it helps others resolve this.

@erusev
Copy link
Owner

erusev commented Mar 15, 2015

I'll see what I can do. It might take some time, though. I'll be quite busy over the next couple of weeks.

@jaswrks
Copy link

jaswrks commented Mar 15, 2015

It might take some time, though.

Copy that :-) I'm a veteran with PHP, but new to Parsedown. Any ideas where I might start debugging this? If so I might be able to submit a PR for this.

@erusev
Copy link
Owner

erusev commented Mar 15, 2015

Sure,

It's in the blockMarkup methods in Parsedown.php.

Parsedown does supports one line block-level elements, but it looks for a closing tag at the end of the line. It doesn't consider the element closed when it doesn't find a closing tag at the end of the line.

You could give it a shot, but it might take me much less time to fix. Unless it's urgent, it might make more sense to wait.

@jaswrks
Copy link

jaswrks commented Mar 15, 2015

@erusev writes...

blockMarkup methods in Parsedown.php

Thank you :-) I'll take a look at this in the next day or so and see what I can make of it. Otherwise, I'll wait patiently as suggested.

@storeman
Copy link
Contributor

I noticed an issue with a self closing element which ended in an endless loop. If an element doesn't have any children, the html should be appended directly and not through processText().

I've fixed this in:
storeman@ddd0d0b

@erusev
Copy link
Owner

erusev commented May 27, 2015

@storeman Can you provide an example that I can use to reproduce the issue?

@storeman
Copy link
Contributor

First,I have to say it only occurs in the fix from @acmitch. The block element problem seems to be resolved.

Just process '<iframe />' gave me this issue.
I wanted to notify you about this, in case you wanted to merge the fix from @acmitch.

@erusev
Copy link
Owner

erusev commented May 27, 2015

@storeman Ah, I see, thanks, I appreciate it.

@storeman
Copy link
Contributor

Had more issues today with the solution from @acmitch. I deleted my fork because i could not find the clear edge case where his solution was breaking my html. One of the issues was, textareas were replicated and the dom-structure was altered. For example:

<textarea></textarea>
<label><input type="checkbox"> <a href="#">Some test</a></label>

Became

<textarea><textarea></textarea></textarea>
<label><input type="checkbox"></label> <a href="#">Some test</a>

It was caused by the surrounding html, but those 13000 lines were to hard to walk through.

@acmitch
Copy link

acmitch commented May 28, 2015

@storeman thanks for the feedback! Found a few issues related to the code block you provided.

<textarea></textarea>
<label><input type="checkbox"> <a href="#">Some test</a></label>

The first issue, as you mentioned above, was that specific elements were causing endless loops. This has been fixed in my latest commit acmitch@468ecc6

It is important to note that all inline or voided HTML elements must be within the $this->textLevelElements and $this->voidElements arrays located within the parsedown library for my fix to work properly.

Therefore, to fix the textarea issue you must add the textarea value to the $this->voidElements array, like such:

 protected $voidElements = array(
    'area', 'base', 'br', 'col', 'command', 'embed', 'hr', 'img', 'input', 'link', 'meta', 'param', 'source', 'textarea'
 );

Once, that has been added you should be good to go (I've already tested and works like a charm)! Maybe @erusev can add the textarea permanently if he gets a chance. FYI: I plan to do a little more work to this library over the next few months. I can't seem to get the script tags working as well as a few small things. I will likely have to change the saveXML back to saveHTML once my PHP is updated to a newer version. You might even want to make that change if your PHP version is 5.3.6 or greater.

@ericlbarnes
Copy link

Here is a failing testcase if it would be of help:

public function testStripping()
{
    $expectedMarkup = '<p><strong>Contact Method:</strong> email</p><p>Test</p><p><em>Some italic text.</em></p>';
    $actualMarkup = (new ParsedownExtra())->text($expectedMarkup);

    $this->assertEquals($expectedMarkup, $actualMarkup);
}

Results:

1) ParsedownExtraTest::testStripping
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<p><strong>Contact Method:</strong> email</p><p>Test</p><p><em>Some italic text.</em></p>'
+'<p><strong>Contact Method:</strong> email</p>'

@erusev
Copy link
Owner

erusev commented Nov 25, 2015

@ericbarnes Thanks, Eric, I appreciate it, hopefully, I'll have some time later this month to address the issue.

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.

7 participants