559303 - [HTML5] Consolidate strBuf and longStrBuf use in the tokenizer

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect, P4)

Description

[Henri Sivonen (:hsivonen)]

It is unnecessary to have two growable buffers in the HTML5 tokenizer. There should be only one growable buffer and a fixed-size buffer for dealing with the case of avoiding losing the data following an unterminated named character reference that is also a prefix of another named character reference when tokenizing an attribute value. (e.g. foo="&notiX")

Comment 1

[Henri Sivonen (:hsivonen)]

Attachment: htmlparser repo patch

Comment 2

[Henri Sivonen (:hsivonen)]

Attachment: m-c patch

Comment 3

[Henri Sivonen (:hsivonen)]

The javadocs that this patch leaves out-of-date are fixed in the last part of bug 489820.

Comment 4

[Henri Sivonen (:hsivonen)]

Comment on attachment 8625699 [details] [diff] [review]
m-c patch

See bug 1029671 comment 36 for the big picture of the whole queue and about landing.

Comment 5

[William Chen [:wchen]]

Comment on attachment 8625698 [details] [diff] [review]
htmlparser repo patch

Review of attachment 8625698 [details] [diff] [review]:
-----------------------------------------------------------------

::: src/nu/validator/htmlparser/impl/Tokenizer.java
@@ +816,5 @@
> +            charRefBuf = newBuf;
> +        }
> +        charRefBuf[charRefBufLen++] = c;
> +    }
> +    

Trailing whitespace here

@@ +831,5 @@
> +                tokenHandler.characters(charRefBuf, 0, charRefBufLen);
> +            }
> +        }
> +    }
> +    

and here.

@@ +3185,3 @@
>                              if ((returnState & DATA_AND_RCDATA_MASK) != 0) {
> +                                for (int i = charRefBufMark; i < charRefBufLen; i++) {
> +                                    appendStrBuf(charRefBuf[i]);

Slightly unrelated, but it looks like we can replace the for loop with the alternate appendStrBuf that takes buffer/offset/length.

@@ +6415,3 @@
>                              if ((returnState & DATA_AND_RCDATA_MASK) != 0) {
> +                                for (int i = charRefBufMark; i < charRefBufLen; i++) {
> +                                    appendStrBuf(charRefBuf[i]);

Same for this for loop.

Comment 6

[William Chen [:wchen]]

Comment on attachment 8625699 [details] [diff] [review]
m-c patch

This stuff should be regenerated after making the changes to the java file.

Comment 7

[Henri Sivonen (:hsivonen)]

Attachment: htmlparser repo patch, review comments addressed

Thanks. Attaching revised patches for the record.

Comment 8

[Henri Sivonen (:hsivonen)]

Attachment: m-c patch, review comments addressed

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7b3c5dd7ad30

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7b3c5dd7ad30