Repository navigation
Markdown in raw HTML stops working after first raw HTML tag #368
Description
Hello,
I'm using Markdown and the "extra" extension to support Markdown in raw HTML div elements with the attribute markdown='1' as explained on the example page: https://pythonhosted.org/Markdown/extensions/extra.html
However, as soon as a raw HTML tag without the "markdown" attribute occurs inside the elment, Markdown will not be processed anymore after the end of that element. If you put any Markdown code after the "Raw html blocks may also be nested." text of the example page, it will not be processed even though it is still in the "markdown='1'" div.
Short example with 3 Markdown uses where the second one is not processed:
<div markdown="1">
Markdown is *active* here.
<div name="RawHtml">
Raw html blocks may also be nested.
</div>
Markdown is *not* active anymore here.
</div>
Markdown is *active again* here.
Activity
fpw commentedon Nov 19, 2014
Additional info: The code actually works as expected on Linux with Python 2.7.6 and python-markdown 2.4.1 but not on OS X with Python 2.7.8 and latest python-markdown from git.
waylan commentedon Nov 19, 2014
Well that's odd. And if that's the case, I'm afraid I have no way of reproducing the error myself, which means I'm not likely to debug it. Especially if it is in the HTML parsing code. That's the most complex part of the parser. Hopefully someone who can reproduce the error will take an rack at it. Thanks for the report.
facelessuser commentedon Nov 19, 2014
I get this as well, but I also get in 2.4.1. I am not running any Python less than 2.7.8...that would seem odd if the Python version was a real factor, but maybe. If I get some time, I may look at it.
fpw commentedon Nov 19, 2014
I've tested it again with Python 2.7.6 and am not getting it on Linux with the latest revision, so it seems to depend on the Python version I guess.
facelessuser commentedon Nov 19, 2014
Oh...that's annoying, but at least that helps narrow down the issue.
waylan commentedon Nov 19, 2014
@facelessuser what OS are you running? My assumption was that the different OS, not the different python version was what was causing the issue. @fpw reported the problem only on OSX (which I can't replicate as I currently don't have access to such a system). I'm just trying to narrow down the exact situation in which the error occurs.
fpw commentedon Nov 19, 2014
@waylan It is OS independent, but seems to depend on the Python version. I'm getting the unexpected behavior with Python 2.7.8 on both OS X and Linux, but not with Python 2.7.6 on either OS X or Linux.
facelessuser commentedon Nov 19, 2014
I have reproduced on OSX. I have also reproduced it on windows with a portable python 2.7.6. So I should say, I haven't not been able to reproduce it. I have been able to patch it for my scenarios, but I am still trying to understand how @waylan and @fpw can have it work with different environments.
fpw commentedon Nov 19, 2014
As far as I understood, @waylan hasn't tested it himself yet. And as this is the first time I'm using a Python project, maybe my testing is wrong - I've cloned the repository from Git and then just ran
python bin/markdown_py -x extrawhile 2.4.1 was still installed. The script looks like it just imports Markdown, maybe it imports 2.4.1 because I haven't actually installed the version I cloned? If that's how Python packages work, I'm sorry for the confusion, I'm a newbie when it comes to Python :)If that's the case, the bug was introduced after 2.4.1 and exists for all Python versions and OSs in the current branch.
facelessuser commentedon Nov 19, 2014
I would create a file called test.py with the following content:
And then drop it in the Python Markdown 2.5.X folder at the same level as the
markdownfolder. Then open a terminal at that location and run the script:Since the
test.pycurrent working directory contains themarkdownlibrary and the current working directory takes priority over the system library path, it it will import that one instead of the installed one.The way the
markdown_pyscript works, it will first look in the bin folder to see if it can findmarkdown, then it will walk the rest of the system paths looking formarkdownand will find the installed version. So probably not the best way to run it.facelessuser commentedon Nov 19, 2014
I was able to isolate the problem in
preprocessors.pyin_nested_markdown_in_html. It appears it wasn't separating the child html tags from the parent content well. Fixing it's indexing caused the tail text to get processed proper.Now I need to run more exhaustive tests, but at least for the case you posted, I am now getting this:
I did this early this morning, so I haven't had a chance to look into it anymore today. Maybe tonight, I will have a chance, maybe not. Depends what I have going on.
I was mainly concerned that I couldn't get get 2.5.1 to work on 2.7.6 Python, but maybe it was universally broken for 2.5.1.
fpw commentedon Nov 19, 2014
@facelessuser Thanks, your test script allowed me to track down the bug. It is indeed not dependent on the OS or the Python version, sorry again for the previous confusion. I've now tested all releases and the bug was introduced between 2.4-final and 2.4.1-final and is still present in the current branch. The Debian package for 2.4 is called 2.4-1, hence my mistake for reporting this as working in 2.4.1. Using your script I've now double-checked all statements in this post :)
facelessuser commentedon Nov 19, 2014
Fantastic, that makes more sense. I couldn't see any code that looked like it would break for only Python > 2.7.6. I think I may check out the changes that were made between those points and match them up with what I patched in 2.5.1 and see if it all makes sense. I probably need to run some tests on some more complicated HTML scenarios to make sure I am not fixing one thing and breaking something else though.
facelessuser commentedon Nov 19, 2014
Sure enough, it was this commit that broke it bdb1bda. But I need to understand what the intention was behind the original change so that both the original issue is fixed and the this new issue is fixed.
facelessuser commentedon Nov 19, 2014
Issue #308 is the origin of this. I will make sure we are not regressing with my changes before I make a pull request.
facelessuser commentedon Nov 19, 2014
FYI, infinite loop returns with my patch, so I am just going to have to dig in and try to fix this stuff once and for all. So much for a quick fix 😞 . I'm not too worried though, I was just hoping for a quick fix
fpw commentedon Nov 19, 2014
Sounds like a good plan, thanks for your effort :)
waylan commentedon Nov 19, 2014
To be clear, I have not actually tested this. My (apparently wrong) assumption that it was OS related was the reason for my statement that I couldn't reproduce the problem -- I haven't actually tried yet.
@facelessuser It looks like you are making good progress on this. @ryneeverett has a pretty good handle on this part of the code. He might be able to help if you hit any roadblocks.
facelessuser commentedon Nov 19, 2014
No worries, I follow the code fine. The current problem was that the behavior was corrected, but the root of the problem was not. I understand the problem; I see the root of the problem, I just need to put together a solution that addresses the root of the problem.
I guess the real issue is things like
<span>1</span><span>2</span>causes the parser to go into an infinite loop. The staring index was the same as the ending index of blocks so it never replaced the html with the placeholder, it just kept reinserting a new placeholder in front of it...so infinite loop.Part of the solution in #308 was to just always increment the index by 1, but now when start != end, we gobbled up an extra block, hence this issue.
So, on the top level, yeah we could just only increment end if end == start. I think on the surface that would fix the issue. But as things stand, this feature doesn't handle multiple HTML blocks on the same line. I think this can be improved. I think we can split the block by tag boundaries and handle them proper and in a more sane way. This is what I want to take a look at. The current issue is pretty easy to solve, but the root of the problem is what I want to tackle.
ryneeverett commentedon Nov 19, 2014
I would not at all be surprised if more bugs were found with
markdown=1, but I'm not sure what the bug is here. This seems to be a duplicate of the initial report in #308 -- a behavior that, while surprising, is technically not a bug since it only occurs when invalid markdown is passed. The<div name="RawHtml">lines in the above examples are not separated by line breaks. See my comment. (The issue bdb1bda fixed was the infinite loop that user reported in a later comment.) Am I missing something here?ryneeverett commentedon Nov 19, 2014
I should add that, if I am correct in my understanding, I would support making the behavior less surprising (or at the very least logging a warning) if we can do so without compromising adherence to the official markdown specifications. There have now been 4 reports of users mistaking this for a bug: (1)myself, (2)nordin of #308, (3)fpw, and (4)facelessuser, and I suspect all but the most obsessively careful readers of Gruber's spec would be surprised.
fpw commentedon Nov 19, 2014
@ryneeverett The snippet above is actually copied from the official example, so add its author to the list ;) https://pythonhosted.org/Markdown/extensions/extra.html
ryneeverett commentedon Nov 19, 2014
Durp. Well I'm pretty sure I'm the author, but this bug has been snuck past waylan quite a few times now, haha.
waylan commentedon Nov 20, 2014
Brief history lesson. Way back when I was first trying to offer a clone of PHP Markdown Extra, I implemented a very basic markdown=1 feature. It only worked in the most basic of situations and not very well. But if gave me feature completeness to say “we have extra too.” @
waylan commentedon Nov 20, 2014
... Others have expanded it since then and I have mostly just taken their contributions as-is. The fact is, I don't use it. I don't really use much HTML at all. The point is, this feature feels like a lot of useless work to me, so I'm happy to let others sweet the details. Today I could care less if we dropped the markdown=1 feature. Personally I like how CommonMark handles parsing inside HTML. But I would prefer that to exist as a third party extension.
facelessuser commentedon Nov 20, 2014
I actually only use it myself for tables on occasions. I think early on I tried to use it for more, but it never quite worked like I would have liked. Maybe what I will do is just patch the issue and see what I feel like doing at another time.
I should take a better look at some of the thinks they have added. I always like seeing the good parts of other implementations, but I really like how easy it is to extend Python Markdown.
Merge pull request #371 from facelessuser/master