Fixed compilation errors in macros. Whitespace handling in Tree Builder. #205

martinrlilja · 2016-04-06T11:57:49Z

Hi!

I’m working on a project which uses html5ever and I noticed that the tree builder handles newlines and whitespace outside of the body and head tags in a slightly odd way.

Take a simple document like this:

<!DOCTYPE html>
<html>
    <head></head>
    <body></body>
</html>

Currently html5ever ignores the newline and indentation of head. For example, the print-rcdom example yields this on the above document:

#Document
    <!DOCTYPE html "" "">
    <html>
        <head>
        #text: \n    
        <body>
            #text: \n\n

So I changed the BeforeHead rule in tree_builder/rules.rs to treat text in the same way AfterHead does.
(As a side note, doing so yielded some errors in the macros crate which I fixed. Those fixes are also part of this pr.)

#Document
    <!DOCTYPE html "" "">
    <html>
        #text: \n    
        <head>
        #text: \n    
        <body>
            #text: \n\n

So there is now a newline and indentation between the html and head tags. Perfect! But there are still two newlines inside the empty body tag. (Both of them exists after the end tag of body.) I’m guessing they come from the AfterBody rule. Which, I guess, is there in case there is text after the body. But it seems odd that it affects a valid document like this.

So without knowing the spec, maybe only text up until the last non-whitespace character should be inserted into the body and the rest should be where it originally occurred in the input stream.

The output should then look like this:

#Document
    <!DOCTYPE html "" "">
    <html>
        #text: \n    
        <head>
        #text: \n    
        <body>
        #text: \n
    #text: \n

Does this seem reasonable?

This change is


        Fixed compilation errors in macros.


        TreeBuilder BeforeHead handles whitespace like AfterHead.

nox · 2016-04-06T12:00:59Z

If you look at the spec, you'll see it tells to do exactly what we do:

https://html.spec.whatwg.org/multipage/syntax.html#the-before-head-insertion-mode

A character token that is one of U+0009 CHARACTER TABULATION, U+000A LINE FEED (LF), U+000C FORM FEED (FF), U+000D CARRIAGE RETURN (CR), or U+0020 SPACE

Ignore the token.

nox · 2016-04-06T12:03:16Z

What was the first commit for, btw?

martinrlilja · 2016-04-06T12:09:07Z

Oh, thanks, should’ve ran the tests :/

When I changed tree_builder/rules.rs it wanted me to compile with cargo build --features codegen which gave a bunch of errors because apparently syntax::Parser functions no longer return a FatalError.


        Reverted last commit.


        Nightly build of the macros crate.

jdm · 2016-04-26T18:36:31Z

r? @nox

jdm · 2016-10-03T16:52:01Z

Shoot, I'm really sorry we dropped the ball on reviewing these changes. This would have avoided #216 (or at least alerted us sooner); as it is, we're replacing the macros in #217, so I'm going to go ahead and close this. Sorry for the frustrating experience!

martinrlilja added 2 commits Apr 5, 2016

Fixed compilation errors in macros.

8ab41e1

TreeBuilder BeforeHead handles whitespace like AfterHead.

Loading status checks…

5125ed2

martinrlilja added 2 commits Apr 6, 2016

Reverted last commit.

cbbb5c2

Nightly build of the macros crate.

Loading status checks…

8150395

jdm closed this Oct 3, 2016

servo / html5ever

Fixed compilation errors in macros. Whitespace handling in Tree Builder. #205

Fixed compilation errors in macros. Whitespace handling in Tree Builder. #205

martinrlilja commented Apr 6, 2016

nox commented Apr 6, 2016

nox commented Apr 6, 2016

martinrlilja commented Apr 6, 2016

jdm commented Apr 26, 2016

jdm commented Oct 3, 2016

servo / html5ever

Join GitHub today

Fixed compilation errors in macros. Whitespace handling in Tree Builder. #205

Fixed compilation errors in macros. Whitespace handling in Tree Builder. #205

Conversation

martinrlilja commented Apr 6, 2016

nox commented Apr 6, 2016

nox commented Apr 6, 2016

martinrlilja commented Apr 6, 2016

jdm commented Apr 26, 2016

jdm commented Oct 3, 2016