Created attachment 58807 [details] One attempt

It appears that after nearly 3 weeks of working with the DocumentParser (formerly Tokenizer) system I still don't understand how the "appendData" parameter of write() is supposed to work. My guess is that the original authors of this code also don't understand what it really does. The attached patch was an attempt to clarify it, but I'm not sure it actually made things less confusing.

I think the confusing part comes mostly because write() is used for 4 things: 1. Network Appends, write(source, true) 2. document.write(), parser.forceSynchronous(); write(source, false) (sometime 'true'!) 3. Synchronous parsing, parser.forceSynchronous(); write(source, true) (sometime 'false'!) 4. Pumping the parser, write("", true) (sometime 'false'!) In the new HTML5 parser we have instead: 1. write(source, true) 2. write(source, false) 3. write(source, false) 4. pumpLexer(ForceSynchronous or AllowYield) Network Appends should always be "true" document.write() should always be "false" synchronous parsing (like innerHTML or responseXML) should always create a new parser so it won't matter, but should probably always be "false" pumping the parser should be a separate concept not requiring going through write() Maybe a better way to fix this would be to add two new replacements for write(): writeFromScript(source) appendFromNetwork(source) And deploy those where possible, keeping write(source, bool) for the confusing cases for now. Not sure.

I think it makes sense to have DocumentParser use two separate functions for this. In general it’s a good design principle to not have individual DocumentParser instances calling the public DocumentParser functions to drive themselves. For one thing, the functions are needlessly virtual. But generally it’s useful to distinguish external entry points from calls within the classes.

We fixed this as part of the HTML parser re-write! *** This bug has been marked as a duplicate of bug 39259 ***