Fix entity name handling in HTML.

Add code to the HTML parser to fix entities that are missing semicolons.
Now, <p>1 &lt 2</p> will be treated as equivalent to <p>1 &lt; </p>.


Submitted @4246

[gagan.goku, 2010-08-12 05:03:00]

Nice change :)

http://codereview.appspot.com/1978041/diff/1/6
File src/com/google/caja/parser/html/Html5ElementStack.java (right):

http://codereview.appspot.com/1978041/diff/1/6#newcode528
src/com/google/caja/parser/html/Html5ElementStack.java:528: +
")(?=$|[^;0-9A-Za-z])"
The not followed needs to be different for &#x100 and &#100

For &#100, the negative lookahead should be "(?![;0-9])" so that &#100a is
treated as da (conformant with what webkit does)
For &#x100, the negative lookahead should be "(?![;0-9a-fA-F])".

http://codereview.appspot.com/1978041/diff/1/2
File tests/com/google/caja/parser/html/DomParserTest.java (right):

http://codereview.appspot.com/1978041/diff/1/2#newcode1251
tests/com/google/caja/parser/html/DomParserTest.java:1251: public final void
testEntities() throws Exception {
You should also add a test for CDATA entities not getting modified.

[MikeSamuel, 2010-08-12 19:02:51]

http://codereview.appspot.com/1978041/diff/1/6
File src/com/google/caja/parser/html/Html5ElementStack.java (right):

http://codereview.appspot.com/1978041/diff/1/6#newcode528
src/com/google/caja/parser/html/Html5ElementStack.java:528: +
")(?=$|[^;0-9A-Za-z])"
On 2010/08/12 05:03:00, gagan.goku wrote:
> The not followed needs to be different for &#x100 and &#100
> 
> For &#100, the negative lookahead should be "(?![;0-9])" so that &#100a is
> treated as da (conformant with what webkit does)
> For &#x100, the negative lookahead should be "(?![;0-9a-fA-F])".

Done.

http://codereview.appspot.com/1978041/diff/1/2
File tests/com/google/caja/parser/html/DomParserTest.java (right):

http://codereview.appspot.com/1978041/diff/1/2#newcode1251
tests/com/google/caja/parser/html/DomParserTest.java:1251: public final void
testEntities() throws Exception {
On 2010/08/12 05:03:00, gagan.goku wrote:
> You should also add a test for CDATA entities not getting modified.

Done.

[gagan.goku, 2010-08-13 00:24:49]

lgtm.

Nice way of testing that entity fixup will not happen inside script tag because
its contents are treated as CDATA.
Is my understand correct that cdata is skipped because in
html5elementStack.java, we do entityFixup only for tokens of type
"HtmlTokenType.TEXT" ?

http://codereview.appspot.com/1978041/diff/7001/2007
File src/com/google/caja/parser/html/Html5ElementStack.java (right):

http://codereview.appspot.com/1978041/diff/7001/2007#newcode523
src/com/google/caja/parser/html/Html5ElementStack.java:523: +
"[0-9]{1,7}(?=$|[^;0-9])"
Isnt "(?![;0-9])" cleaner ?

[MikeSamuel, 2010-08-13 01:25:54]

On 2010/08/13 00:24:49, gagan.goku wrote:
> lgtm.
> 
> Nice way of testing that entity fixup will not happen inside script tag
because
> its contents are treated as CDATA.
> Is my understand correct that cdata is skipped because in
> html5elementStack.java, we do entityFixup only for tokens of type
> "HtmlTokenType.TEXT" ?

Yes.

> http://codereview.appspot.com/1978041/diff/7001/2007
> File src/com/google/caja/parser/html/Html5ElementStack.java (right):
> 
> http://codereview.appspot.com/1978041/diff/7001/2007#newcode523
> src/com/google/caja/parser/html/Html5ElementStack.java:523: +
> "[0-9]{1,7}(?=$|[^;0-9])"
> Isnt "(?![;0-9])" cleaner ?

Done.

[Jasvir, 2010-08-16 19:38:32]

LGTM++

http://codereview.appspot.com/1978041/diff/10001/11007
File src/com/google/caja/plugin/ExpressionSanitizerCaja.java (right):

http://codereview.appspot.com/1978041/diff/10001/11007#newcode64
src/com/google/caja/plugin/ExpressionSanitizerCaja.java:64: result =
newES53Rewriter().expand(input);
Fine.

http://codereview.appspot.com/1978041/diff/10001/11008
File src/com/google/caja/reporting/MessageType.java (right):

http://codereview.appspot.com/1978041/diff/10001/11008#newcode45
src/com/google/caja/reporting/MessageType.java:45: "%s: HTML entity missing
closing semicolon %s", MessageLevel.LINT),
We're correcting semantics of malformed code - MessageLevel.WARNING

[MikeSamuel, 2010-08-16 19:46:25]

http://codereview.appspot.com/1978041/diff/10001/11008
File src/com/google/caja/reporting/MessageType.java (right):

http://codereview.appspot.com/1978041/diff/10001/11008#newcode45
src/com/google/caja/reporting/MessageType.java:45: "%s: HTML entity missing
closing semicolon %s", MessageLevel.LINT),
On 2010/08/16 19:38:32, jasvir wrote:
> We're correcting semantics of malformed code - MessageLevel.WARNING

Done.

[gagan.goku, 2010-08-17 07:31:02]

Thanks for the fix Mike.