From bb1bca7058e6a14a7b74354933c0c805ca89bb72 Mon Sep 17 00:00:00 2001 From: sideshowbarker Date: Mon, 9 Nov 2026 13:37:43 +0900 Subject: [PATCH] fix: Let tree-building parses report the locations streaming ones do MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem: In the Nu HTML Checker, with --no-stream, each HTML parser message got a range that started at the start of the document — or at the end of the previous message — so its extract and highlight took in all the markup from there to the error. And a checker message reported at an end tag got no location at all. Cause: When its streamability violation policy is ALLOW, HtmlParser uses its SAXTreeBuilder, which builds the whole tree before TreeParser replays it as SAX events. So a SAX consumer that tracks locations had none for any error the parser reported, which the checker needs to find where the range of each error starts. Also, SAXTreeBuilder never set an element’s end locator, so the replay gave endElement events no location. Fix: As SAXTreeBuilder builds the tree, it now sends the events a streaming parse emits, at the same points, to any handlers set with the new HtmlParser.setTreeBuildingWiretap() — so those handlers get the same locations a streaming parse gives, interleaved with the parser’s errors. SAXTreeBuilder also now sets the end locator of each element it pops. Fixes https://github.com/validator/validator/issues/2168 --- .../validator/htmlparser/sax/HtmlParser.java | 29 +++++++ .../htmlparser/sax/SAXTreeBuilder.java | 77 +++++++++++++++++-- 2 files changed, 101 insertions(+), 5 deletions(-) diff --git a/src/nu/validator/htmlparser/sax/HtmlParser.java b/src/nu/validator/htmlparser/sax/HtmlParser.java index 8ff8167d..44d274b4 100644 --- a/src/nu/validator/htmlparser/sax/HtmlParser.java +++ b/src/nu/validator/htmlparser/sax/HtmlParser.java @@ -99,6 +99,10 @@ public class HtmlParser implements XMLReader { private LexicalHandler lexicalHandler = null; + private ContentHandler treeBuildingContentHandler = null; + + private LexicalHandler treeBuildingLexicalHandler = null; + private DTDHandler dtdHandler = null; private EntityResolver entityResolver = null; @@ -172,6 +176,8 @@ private void lazyInit() { if (driver == null) { if (streamabilityViolationPolicy == XmlViolationPolicy.ALLOW) { this.saxTreeBuilder = new SAXTreeBuilder(); + this.saxTreeBuilder.setWiretap(treeBuildingContentHandler, + treeBuildingLexicalHandler); this.treeBuilder = this.saxTreeBuilder; this.saxStreamer = null; this.driver = new Driver(newTokenizer(treeBuilder, true)); @@ -571,6 +577,29 @@ public void setLexicalHandler(LexicalHandler handler) { } } + /** + * Sets handlers that get events while the parser builds a tree (that is, + * when the streamability violation policy is ALLOW): The + * events a streaming parse would emit, at the same points in the input, + * and with the tokenizer as their locator. So they report the same + * locations as a streaming parse does, interleaved with any errors + * reported to the error handler. They don't form a well-formed SAX + * stream, since the tree builder can move nodes after creating them; + * the handlers set with setContentHandler and + * setLexicalHandler get the events for the finished tree. + * + * @param contentHandler the content handler, or null + * @param lexicalHandler the lexical handler, or null + */ + public void setTreeBuildingWiretap(ContentHandler contentHandler, + LexicalHandler lexicalHandler) { + treeBuildingContentHandler = contentHandler; + treeBuildingLexicalHandler = lexicalHandler; + if (saxTreeBuilder != null) { + saxTreeBuilder.setWiretap(contentHandler, lexicalHandler); + } + } + /** * @see org.xml.sax.XMLReader#setDTDHandler(org.xml.sax.DTDHandler) */ diff --git a/src/nu/validator/htmlparser/sax/SAXTreeBuilder.java b/src/nu/validator/htmlparser/sax/SAXTreeBuilder.java index 06378e2b..4a9cc2a8 100644 --- a/src/nu/validator/htmlparser/sax/SAXTreeBuilder.java +++ b/src/nu/validator/htmlparser/sax/SAXTreeBuilder.java @@ -23,7 +23,9 @@ package nu.validator.htmlparser.sax; +import org.xml.sax.ContentHandler; import org.xml.sax.SAXException; +import org.xml.sax.ext.LexicalHandler; import nu.validator.htmlparser.impl.HtmlAttributes; import nu.validator.htmlparser.impl.TreeBuilder; @@ -45,23 +47,53 @@ class SAXTreeBuilder extends TreeBuilder { private Node cachedTablePreviousSibling = null; + private ContentHandler wiretapContentHandler = null; + + private LexicalHandler wiretapLexicalHandler = null; + SAXTreeBuilder() { super(); } + /** + * Sets handlers that get, while the tree is being built, the events a + * streaming parse would emit at the same points, with the tokenizer as + * their locator. + * + * @param contentHandler the content handler, or null + * @param lexicalHandler the lexical handler, or null + */ + void setWiretap(ContentHandler contentHandler, + LexicalHandler lexicalHandler) { + wiretapContentHandler = contentHandler; + wiretapLexicalHandler = lexicalHandler; + } + @Override - protected void appendComment(Element parent, char[] buf, int start, int length) { + protected void appendComment(Element parent, char[] buf, int start, int length) + throws SAXException { parent.appendChild(new Comment(tokenizer, buf, start, length)); + if (wiretapLexicalHandler != null) { + wiretapLexicalHandler.comment(buf, start, length); + } } @Override - protected void appendCommentToDocument(char[] buf, int start, int length) { + protected void appendCommentToDocument(char[] buf, int start, int length) + throws SAXException { document.appendChild(new Comment(tokenizer, buf, start, length)); + if (wiretapLexicalHandler != null) { + wiretapLexicalHandler.comment(buf, start, length); + } } @Override - protected void appendCharacters(Element parent, char[] buf, int start, int length) { + protected void appendCharacters(Element parent, char[] buf, int start, int length) + throws SAXException { parent.appendChild(new Characters(tokenizer, buf, start, length)); + if (wiretapContentHandler != null) { + wiretapContentHandler.characters(buf, start, length); + } } @Override @@ -91,10 +123,15 @@ protected void addAttributesToElement(Element element, HtmlAttributes attributes * @see nu.validator.htmlparser.impl.TreeBuilder#appendDoctypeToDocument(java.lang.String, java.lang.String, java.lang.String) */ @Override - protected void appendDoctypeToDocument(String name, String publicIdentifier, String systemIdentifier) { + protected void appendDoctypeToDocument(String name, String publicIdentifier, String systemIdentifier) + throws SAXException { DTD dtd = new DTD(tokenizer, name, publicIdentifier, systemIdentifier); dtd.setEndLocator(tokenizer); document.appendChild(dtd); + if (wiretapLexicalHandler != null) { + wiretapLexicalHandler.startDTD(name, publicIdentifier, systemIdentifier); + wiretapLexicalHandler.endDTD(); + } } /** @@ -124,16 +161,43 @@ protected void end() throws SAXException { document.setEndLocator(tokenizer); cachedTable = null; cachedTablePreviousSibling = null; + if (wiretapContentHandler != null) { + wiretapContentHandler.endDocument(); + } } /** * @see nu.validator.htmlparser.impl.TreeBuilder#start() */ @Override - protected void start(boolean fragment) { + protected void start(boolean fragment) throws SAXException { document = new Document(tokenizer); cachedTable = null; cachedTablePreviousSibling = null; + if (wiretapContentHandler != null) { + wiretapContentHandler.setDocumentLocator(tokenizer); + if (!fragment) { + wiretapContentHandler.startDocument(); + } + } + } + + @Override + protected void elementPushed(String ns, String name, Element node) + throws SAXException { + if (wiretapContentHandler != null) { + wiretapContentHandler.startElement(ns, name, name, + node.getAttributes()); + } + } + + @Override + protected void elementPopped(String ns, String name, Element node) + throws SAXException { + node.setEndLocator(tokenizer); + if (wiretapContentHandler != null) { + wiretapContentHandler.endElement(ns, name, name); + } } @Override @@ -172,6 +236,9 @@ protected Element createAndInsertFosterParentedElement(String ns, String name, } else { stackParent.appendChild(child); } + if (wiretapContentHandler != null) { + wiretapContentHandler.characters(buf, start, length); + } } @Override protected void insertFosterParentedChild(Element child,