Fix handling of 'target' attributes (issue 6240043)

[ihab...@gmail.com]

Reviewers: felix8a,

Description:
Fixes http://code.google.com/p/google-caja/issues/detail?id=1471

This change required passing an out-of-band "this attribute was
unspecified by the guest programmer" signal from TemplateCompiler to
HtmlAttributeRewriter. See changes for details of how this is done.

Please review this at http://codereview.appspot.com/6240043/

Affected files:
M src/com/google/caja/plugin/domado.js
M src/com/google/caja/plugin/templates/HtmlAttributeRewriter.java
M src/com/google/caja/plugin/templates/TemplateCompiler.java
M tests/com/google/caja/plugin/HtmlCompiledPluginTest.java
M tests/com/google/caja/plugin/es53-test-target-attribute-presets.js
M tests/com/google/caja/plugin/templates/TemplateCompilerTest.java
M
tests/com/google/caja/plugin/templates/template-compiler-golden1-dynamic.js
M
tests/com/google/caja/plugin/templates/template-compiler-golden1-nulluripol.js
M
tests/com/google/caja/plugin/templates/template-compiler-golden1-static.js
M tests/com/google/caja/service/HtmlHandlerTest.java

[fel...@gmail.com]

what if I say <a target> or <a target=> ?
does this do the right thing?


http://codereview.appspot.com/6240043/

[ihab...@gmail.com]

http://codereview.appspot.com/6240043/

[ihab...@gmail.com]

> <a target>

Added a test.

> <a target=>

FATAL_ERROR in the server side cajoler.

http://codereview.appspot.com/6240043/

[fel...@gmail.com]

On 2012/05/23 22:53:58, ihab.awad wrote:
> > <a target=>

> FATAL_ERROR in the server side cajoler.

that seems wrong. the cajoler currently doesn't complain about that,
and I can find many instances of people writing target="", though it's
not clear to me what the intent of that is.

http://codereview.appspot.com/6240043/

[ihab...@gmail.com]

(target="") !== (target=)

I verified in the playground. What makes you think the cajoler doesn't complain?

--
Ihab A.B. Awad, Palo Alto, CA

[felix]

On Wed, May 23, 2012 at 4:55 PM, <ihab...@gmail.com> wrote:

(target="") !== (target=)

I verified in the playground. What makes you think the cajoler doesn't complain?

I tried

  ​<a target=>hi</a>

at caja.appspot.com and it worked for me.

[ihab...@gmail.com]

On Wed, May 23, 2012 at 4:59 PM, felix <fel...@gmail.com> wrote:

I tried

  ​<a target=>hi</a>

at caja.appspot.com and it worked for me.

Oh weird. It worked for me too. What did not work for me was:

​<a target= href="http://www.google.com/">hi</a>​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​

and it complained with:

FATAL_ERROR unknown:///unknown:0+0: malformed xhtml: "http://www.google.com/":

unknown:///unknown:0: <a target= href="http://www.google.com/">hi</a>

so it seems it parses the "href=..." as the possible value of the "target" attribute. Fwiw, the following does work (again, as with your case "target=" is the last thing inside the angle brackets):

​

<a href="http://www.google.com/" target=>hi</a>​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​

Ok, I'll add your case to the tests and resend.

Ihab

[felix]

On Wed, May 23, 2012 at 5:11 PM, <ihab...@gmail.com> wrote:

On Wed, May 23, 2012 at 4:59 PM, felix <fel...@gmail.com> wrote:

I tried

  ​<a target=>hi</a>

at caja.appspot.com and it worked for me.

Oh weird. It worked for me too. What did not work for me was:

​<a target= href="http://www.google.com/">hi</a>​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​

and it complained with:

FATAL_ERROR unknown:///unknown:0+0: malformed xhtml: "http://www.google.com/":

unknown:///unknown:0: <a target= href="http://www.google.com/">hi</a>

ah, yeah, iirc that's a weirdness with variant html parsing by old browsers, which should be mostly irrelevant now, and caja is being weird about what it accepts.

[ihab...@gmail.com]

http://codereview.appspot.com/6240043/

[ihab...@gmail.com]

Test added per discussion.

http://codereview.appspot.com/6240043/

[fel...@gmail.com]

one nit, and lgtm


http://codereview.appspot.com/6240043/diff/10002/src/com/google/caja/plugin/templates/HtmlAttributeRewriter.java
File src/com/google/caja/plugin/templates/HtmlAttributeRewriter.java
(right):

http://codereview.appspot.com/6240043/diff/10002/src/com/google/caja/plugin/templates/HtmlAttributeRewriter.java#newcode366
src/com/google/caja/plugin/templates/HtmlAttributeRewriter.java:366:
return (Expression) QuasiBuilder.substV(""
fix indentation

http://codereview.appspot.com/6240043/

[ihab...@gmail.com]

http://codereview.appspot.com/6240043/

[fel...@gmail.com]

still lgtm

http://codereview.appspot.com/6240043/

[ihab...@gmail.com]

http://codereview.appspot.com/6240043/

[ihab...@gmail.com]

@4887


http://codereview.appspot.com/6240043/