msg157890 - (view) |
Author: Michel Leunen (Michel.Leunen) |
Date: 2012-04-09 20:01 |
HTMLParser fails to parse this structure of tags:
'<a></a><script></script><meta><meta / ><body></body>'
Parsing stops after the first meta tag ignoring the remainers
from HTMLParser import HTMLParser
parser = process_html()
parser.feed('<a></a><script></script><meta><meta / ><body></body>')
Python 2.7.2+ Ubuntu 11.10
|
msg157899 - (view) |
Author: Jim Jewett (Jim.Jewett) *  |
Date: 2012-04-09 20:50 |
What do you think it should do?
My thought is that meta tags may or may not be void, but certainly should not be nested. As XML, I would parse that as
<a></a>
<script></script>
<meta>
<meta / >
<body></body>
*missing closing tag
But for html, there is more cleanup. The catch is that this module probably can't keep up with BeautifulSoup or HTML5lib ... is this particular tag situation common enough to be even have a defined handling, let alone one that is worth adding to a "simple" module?
|
msg157906 - (view) |
Author: Ezio Melotti (ezio.melotti) *  |
Date: 2012-04-09 21:39 |
With Python 2.7.3rc2 and 3.3.0a0 (strict=False) I get:
Start tag: a
End tag : a
Start tag: script
End tag : script
Start tag: meta
Data : <meta / >
Start tag: body
End tag : body
This is better, but still not 100% correct, the "<meta / >" shouldn't be seen as data.
|
msg158109 - (view) |
Author: Georg Brandl (georg.brandl) *  |
Date: 2012-04-12 07:22 |
ISTM that "<meta / >" is neither valid HTML nor valid XHTML.
|
msg158111 - (view) |
Author: Ezio Melotti (ezio.melotti) *  |
Date: 2012-04-12 07:48 |
Here's a patch.
|
msg158130 - (view) |
Author: Jim Jewett (Jim.Jewett) *  |
Date: 2012-04-12 14:03 |
-1 on that particular patch.
<tagname / >
(with only whitespace between "/" and ">") strikes me as obviously intending to close the tag, and a reasonably common error.
I can't think of any reason to support nested meta tags while not supporting sloppy self-closing tags.
|
msg158132 - (view) |
Author: Jim Jewett (Jim.Jewett) *  |
Date: 2012-04-12 14:09 |
This issue is also marked for (bugfix-only) 2.7 and 3.2.
Unless there is a specification somewhere (or at least an editor's draft), I can't really see any particular parse as a bugfix. Was the goal just to make the parse finish, as opposed to stopping part way through the text?
|
msg158140 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2012-04-12 14:58 |
To be consistent, this patch should remove the references to http://www.w3.org/TR/html5/tokenization.html#tag-open-state and http://www.w3.org/TR/html5/tokenization.html#tag-open-state as irrelevant.
|
msg158143 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2012-04-12 15:26 |
Yes, after considerable discussion those of working on this stuff decided that the goal should be that the parser be able to complete parsing, without error, anything the typical browsers can parse (which means, pretty much anything, though that says nothing about whether the result of the parse is useful in any way). In other words, we've been treating it as a bug when the parser throws an error, since one generally uses the library to parse web pages from the internet and having the parse fail leaves you SOL for doing anything useful with the bad pages one gets therefrom. (Note that if the parser was doing strict adherence to the older RFCs our decision would have been different...but it is not. It has always accepted *some* badly formed documents, and rejected others.)
Also note that BeautifulSoup in Python2 used the sgml parser, which didn't throw errors, but that is gone in Python3. In Python3 BeautifulSoup uses the html parser...which is what started us down this road to begin with.
|
msg158168 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2012-04-12 21:03 |
I apologize for misplaced sarcasm. After more careful reading of the source code, I found out that the patch really meets the specifications and the behavior of all tested me browsers. Despite its awkward appearance, the patch fixes a flaw of the original code for this corner case. But it allows the passage of an invalid code in strict mode. I think, this problem can be solved by a more direct way, perhaps even simplifying the original code.
|
msg158185 - (view) |
Author: Ezio Melotti (ezio.melotti) *  |
Date: 2012-04-12 23:26 |
Attached a new patch with a few more tests and a simplification of the attrfind regex.
> But it allows the passage of an invalid code in strict mode.
HTMLParser is following the HTML5 specs, and doesn't do validation, so there's no strict mode (and the strict mode of Python 3 will be removed/deprecated soon).
> I think, this problem can be solved by a more direct way,
> perhaps even simplifying the original code.
This might be true, but I'm quite happy with this patch for now. Maybe one day I'll rewrite it.
> Unless there is a specification somewhere (or at least an editor's
> draft), I can't really see any particular parse as a bugfix.
If HTMLParser doesn't parse as the HTML5 specs say, then it's considered a bug.
|
msg158195 - (view) |
Author: Jim Jewett (Jim.Jewett) *  |
Date: 2012-04-13 02:55 |
On Thu, Apr 12, 2012 at 7:26 PM, Ezio Melotti wrote:
> If HTMLParser doesn't parse as the HTML5 specs say,
> then it's considered a bug.
I had thought that was the goal of the html5lib, and that HTMLParser
was explicitly aiming at a much reduced model in order to remain
simple. (And also because the HTML5 spec is still changing fairly
often, particularly compared to the lifecycle of a feature release of
python.)
|
msg158198 - (view) |
Author: Ezio Melotti (ezio.melotti) *  |
Date: 2012-04-13 06:20 |
HTMLParser is still simpler than html5lib, but if/when possible we are following the HTML5 standard rather than taking arbitrary decisions (like we used to do before HTML5). HTMLParser doesn't claim to be a fully compliant HTML5 parser (and probably never will) -- its goal is rather being able to parse real world HTML in the same way browsers do. There are also a couple of corner cases that are not implemented in HTMLParser because it would make the code too complicated for a little gain.
|
msg158215 - (view) |
Author: Jim Jewett (Jim.Jewett) *  |
Date: 2012-04-13 17:34 |
It sounds like this is a case where the docs should mention an external library; perhaps something like changing the intro of http://docs.python.org/dev/library/html.parser.html from:
"""
19.2. html.parser — Simple HTML and XHTML parser
Source code: Lib/html/parser.py
This module defines a class HTMLParser which serves as the basis for parsing text files formatted in HTML (HyperText Mark-up Language) and XHTML.
"""
to:
"""
19.2. html.parser — Simple HTML and XHTML parser
Source code: Lib/html/parser.py
This module defines a class HTMLParser which serves as the basis for parsing text files formatted in HTML (HyperText Mark-up Language) and XHTML.
Note that mainstream web browsers also attempt to repair invalid markup; the algorithms for this can be quite complex, and are evolving too quickly for the Python release cycle. Applications handling arbitrary web pages should consider using 3rd-party modules. The python version of html5lib ( http://code.google.com/p/html5lib/ ) is being developed in parallel with the HTML standard itself, and serves as a reference implementation.
"""
|
msg158220 - (view) |
Author: Éric Araujo (eric.araujo) *  |
Date: 2012-04-13 17:50 |
Sure, the docs should explain better that html.parser tries its best to parse stuff, is not a validating parser, and is actively developed, contrary to the popular belief that standard library modules never get improved. I’m less sure about links, there is more than one popular library (html5lib, lxml, BeautifulSoup). Another bug needs to be opened, we’re off-topic for this one — but thanks for the remark and proposed patch.
|
msg158695 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-04-19 01:36 |
New changeset 36c901fcfcda by Ezio Melotti in branch '2.7':
#14538: HTMLParser can now parse correctly start tags that contain a bare /.
http://hg.python.org/cpython/rev/36c901fcfcda
New changeset ba4baaddac8d by Ezio Melotti in branch '3.2':
#14538: HTMLParser can now parse correctly start tags that contain a bare /.
http://hg.python.org/cpython/rev/ba4baaddac8d
New changeset 0f837071fd97 by Ezio Melotti in branch 'default':
#14538: merge with 3.2.
http://hg.python.org/cpython/rev/0f837071fd97
|
msg158696 - (view) |
Author: Ezio Melotti (ezio.melotti) *  |
Date: 2012-04-19 01:45 |
This is now fixed, thanks for the report!
Regarding the documentation feel free to open another issue, but at some point I'll probably update it anyway and/or write down somewhere what the future plans for HTMLParser and its goals.
|
msg158718 - (view) |
Author: Michel Leunen (Michel.Leunen) |
Date: 2012-04-19 11:55 |
Thanks guys for your comments and for solving this issue. Great work!
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:28 | admin | set | github: 58743 |
2012-04-19 11:55:29 | Michel.Leunen | set | messages:
+ msg158718 |
2012-04-19 01:45:30 | ezio.melotti | set | status: open -> closed resolution: fixed messages:
+ msg158696
stage: patch review -> resolved |
2012-04-19 01:36:30 | python-dev | set | nosy:
+ python-dev messages:
+ msg158695
|
2012-04-13 17:50:31 | eric.araujo | set | nosy:
+ eric.araujo messages:
+ msg158220
|
2012-04-13 17:34:02 | Jim.Jewett | set | messages:
+ msg158215 |
2012-04-13 06:20:33 | ezio.melotti | set | messages:
+ msg158198 |
2012-04-13 02:55:34 | Jim.Jewett | set | messages:
+ msg158195 |
2012-04-12 23:26:37 | ezio.melotti | set | files:
+ issue14538-2.diff
messages:
+ msg158185 |
2012-04-12 21:03:52 | serhiy.storchaka | set | messages:
+ msg158168 |
2012-04-12 15:26:45 | r.david.murray | set | nosy:
+ r.david.murray messages:
+ msg158143
|
2012-04-12 14:58:19 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages:
+ msg158140
|
2012-04-12 14:09:12 | Jim.Jewett | set | messages:
+ msg158132 |
2012-04-12 14:03:56 | Jim.Jewett | set | messages:
+ msg158130 |
2012-04-12 07:48:31 | ezio.melotti | set | files:
+ issue14538.diff keywords:
+ patch messages:
+ msg158111
stage: test needed -> patch review |
2012-04-12 07:22:31 | georg.brandl | set | nosy:
+ georg.brandl messages:
+ msg158109
|
2012-04-09 21:39:38 | ezio.melotti | set | assignee: ezio.melotti stage: test needed messages:
+ msg157906 versions:
+ Python 3.2, Python 3.3 |
2012-04-09 21:19:23 | pitrou | set | nosy:
+ ezio.melotti
|
2012-04-09 20:50:55 | Jim.Jewett | set | nosy:
+ Jim.Jewett messages:
+ msg157899
|
2012-04-09 20:04:38 | Michel.Leunen | set | title: HTMLParser -> HTMLParser: parsing error |
2012-04-09 20:01:55 | Michel.Leunen | create | |