Preserve XMLNS declarations in the DOM Parser

http://code.google.com/p/google-caja/issues/detail?id=1115

There've been a number of bugs pointing out that we remove XMLNS declarations in the output unless
there's an element that uses them.

So
  <div xmlns:svg="..."></div>
is rendered as
  <div></div>

This doesn't seem an error to me, but this change preserves XMLNS
declarations in the DOM and allows the renderer to output them as long
as they don't mask an existing declaration.

I'm not convinced this is a good idea though.
Some have argued that they are necessary for scripts.  E.g. that
  <div xmlns:foo="bar" id="baz"></div>
will behave differently when
  document.getElementById('baz').innerHTML = '<foo:boo/>';

I don't believe this.  Does anyone have a reference to a standard or an
actual testcase that demonstrates this.

[chirag, 2010-09-20 23:39:14]

lgtm. I don't have a reference for that specific case, but the
opensocial-templates spec uses xmlns declarations in script tags.

http://opensocial-resources.googlecode.com/svn/spec/1.0/OpenSocial-Templating...

[MikeSamuel, 2010-09-21 01:15:31]

2010/9/20  <chiragshah1@gmail.com>:
> lgtm. I don't have a reference for that specific case, but the
> opensocial-templates spec uses xmlns declarations in script tags.

How are they used?
The only description of xmlns in that doc is talking about xmlns as
used to parse an XML document.  Nothing about why xmlns would need to
be preserved in a document after it's been parsed.

>
http://opensocial-resources.googlecode.com/svn/spec/1.0/OpenSocial-Templating...
>
>
>
> http://codereview.appspot.com/2183044/

[chirag, 2010-09-21 23:10:33]

After shindig converts script tags with type=os/* to equivalent OSML tags, it
will re-parse each newly converted OSML tag within the correct namespace.

Here's the end-to-end flow. 3b starts to explain how xmlns is used:
1) Developer writes the following app:
<script type="text/os-template" xmlns:foo="http://example.com"
xmlns:os="http://ns.opensocial.org/2008/markup"><foo:Bar attr='Hello'/></script>

2) Shindig parses the document (Neko/Caja) and ends up with a script node
containing a text child.

3) Since the script's child is a text node,
GadgetHtmlParser#reprocessScriptForOpenSocial() converts the opensocial script
tag to an osml tag  and passes the following fragment to Neko for reprocessing.
<OSTemplate xmlns:foo="http://example.com"
xmlns:os="http://ns.opensocial.org/2008/markup"><foo:Bar
attr='Hello'/></OSTemplate>

3b) When the CajaHtmlParser is used the following fragment gets passed for
reprocessing:
<OSTemplate><foo:Bar attr='Hello'/></OSTemplate>

Since xmlns:foo was eliminated,
doc.getElementsByTagName("foo:Bar").item(0).getNamespaceURI() returns
"unknown://foo" instead of http://example.com.

This behavior causes 
org.apache.shindig.gadgets.templates.tags.DefaultTagRegistry#getHandlerFor() to
return null because it needs to find appropriate tag handlers based on the
namespaceUri.

4) This newly processed fragment replaces the original script node.

This code review in shindig explains this process in more detail:
http://codereview.appspot.com/2200046/

On 2010/09/21 01:15:31, MikeSamuel wrote:
> 2010/9/20  <chiragshah1@gmail.com>:
> > lgtm. I don't have a reference for that specific case, but the
> > opensocial-templates spec uses xmlns declarations in script tags.
> 
> How are they used?
> The only description of xmlns in that doc is talking about xmlns as
> used to parse an XML document.  Nothing about why xmlns would need to
> be preserved in a document after it's been parsed.
> 
> >
>
http://opensocial-resources.googlecode.com/svn/spec/1.0/OpenSocial-Templating...
> >
> >
> >
> > http://codereview.appspot.com/2183044/
>

[MikeSamuel, 2010-09-21 23:42:55]

2010/9/21  <chiragshah1@gmail.com>:
> After shindig converts script tags with type=os/* to equivalent OSML
> tags, it will re-parse each newly converted OSML tag within the correct
> namespace.
>
> Here's the end-to-end flow. 3b starts to explain how xmlns is used:
> 1) Developer writes the following app:
> <script type="text/os-template" xmlns:foo="http://example.com"
> xmlns:os="http://ns.opensocial.org/2008/markup"><foo:Bar
> attr='Hello'/></script>
>
> 2) Shindig parses the document (Neko/Caja) and ends up with a script
> node containing a text child.
>
> 3) Since the script's child is a text node,
> GadgetHtmlParser#reprocessScriptForOpenSocial() converts the opensocial
> script tag to an osml tag  and passes the following fragment to Neko for
> reprocessing.
> <OSTemplate xmlns:foo="http://example.com"
> xmlns:os="http://ns.opensocial.org/2008/markup"><foo:Bar
> attr='Hello'/></OSTemplate>
>
> 3b) When the CajaHtmlParser is used the following fragment gets passed
> for reprocessing:
> <OSTemplate><foo:Bar attr='Hello'/></OSTemplate>
>
> Since xmlns:foo was eliminated,
> doc.getElementsByTagName("foo:Bar").item(0).getNamespaceURI() returns
> "unknown://foo" instead of http://example.com.
>
> This behavior causes
> org.apache.shindig.gadgets.templates.tags.DefaultTagRegistry#getHandlerFor()
> to return null because it needs to find appropriate tag handlers based
> on the namespaceUri.
>
> 4) This newly processed fragment replaces the original script node.
>
> This code review in shindig explains this process in more detail:
> http://codereview.appspot.com/2200046/

Ok, so nothing in that flow requires rendering xmlns declarations.
Only that they be available as attributes in the DOM so they can be
migrated to the manufactured top level
  <os:templates>
element.

Is that right?

> On 2010/09/21 01:15:31, MikeSamuel wrote:
>>
>> 2010/9/20  <chiragshah1@gmail.com>:
>> > lgtm. I don't have a reference for that specific case, but the
>> > opensocial-templates spec uses xmlns declarations in script tags.
>
>> How are they used?
>> The only description of xmlns in that doc is talking about xmlns as
>> used to parse an XML document.  Nothing about why xmlns would need to
>> be preserved in a document after it's been parsed.
>
>> >
>
>
http://opensocial-resources.googlecode.com/svn/spec/1.0/OpenSocial-Templating...
>>
>> >
>> >
>> >
>> > http://codereview.appspot.com/2183044/
>
>
>
>
> http://codereview.appspot.com/2183044/
>

[chirag, 2010-09-22 00:44:39]

Yeah, that is correct when we're rendering server-side templates in shindig.

When the document is serialized and presented to the browser, could a
client-side template parser [1][2] need the xmlns attribute? Just
curious about this use-case and am not sure if it's widely used.

[1] - http://code.google.com/p/google-jstemplate/
[2] -
http://svn.apache.org/viewvc/shindig/trunk/features/src/main/javascript/featu...

On Tue, Sep 21, 2010 at 4:42 PM, Mike Samuel <mikesamuel@gmail.com> wrote:
> 2010/9/21  <chiragshah1@gmail.com>:
>> After shindig converts script tags with type=os/* to equivalent OSML
>> tags, it will re-parse each newly converted OSML tag within the correct
>> namespace.
>>
>> Here's the end-to-end flow. 3b starts to explain how xmlns is used:
>> 1) Developer writes the following app:
>> <script type="text/os-template" xmlns:foo="http://example.com"
>> xmlns:os="http://ns.opensocial.org/2008/markup"><foo:Bar
>> attr='Hello'/></script>
>>
>> 2) Shindig parses the document (Neko/Caja) and ends up with a script
>> node containing a text child.
>>
>> 3) Since the script's child is a text node,
>> GadgetHtmlParser#reprocessScriptForOpenSocial() converts the opensocial
>> script tag to an osml tag  and passes the following fragment to Neko for
>> reprocessing.
>> <OSTemplate xmlns:foo="http://example.com"
>> xmlns:os="http://ns.opensocial.org/2008/markup"><foo:Bar
>> attr='Hello'/></OSTemplate>
>>
>> 3b) When the CajaHtmlParser is used the following fragment gets passed
>> for reprocessing:
>> <OSTemplate><foo:Bar attr='Hello'/></OSTemplate>
>>
>> Since xmlns:foo was eliminated,
>> doc.getElementsByTagName("foo:Bar").item(0).getNamespaceURI() returns
>> "unknown://foo" instead of http://example.com.
>>
>> This behavior causes
>> org.apache.shindig.gadgets.templates.tags.DefaultTagRegistry#getHandlerFor()
>> to return null because it needs to find appropriate tag handlers based
>> on the namespaceUri.
>>
>> 4) This newly processed fragment replaces the original script node.
>>
>> This code review in shindig explains this process in more detail:
>> http://codereview.appspot.com/2200046/
>
> Ok, so nothing in that flow requires rendering xmlns declarations.
> Only that they be available as attributes in the DOM so they can be
> migrated to the manufactured top level
>  <os:templates>
> element.
>
> Is that right?
>
>> On 2010/09/21 01:15:31, MikeSamuel wrote:
>>>
>>> 2010/9/20  <chiragshah1@gmail.com>:
>>> > lgtm. I don't have a reference for that specific case, but the
>>> > opensocial-templates spec uses xmlns declarations in script tags.
>>
>>> How are they used?
>>> The only description of xmlns in that doc is talking about xmlns as
>>> used to parse an XML document.  Nothing about why xmlns would need to
>>> be preserved in a document after it's been parsed.
>>
>>> >
>>
>>
http://opensocial-resources.googlecode.com/svn/spec/1.0/OpenSocial-Templating...
>>>
>>> >
>>> >
>>> >
>>> > http://codereview.appspot.com/2183044/
>>
>>
>>
>>
>> http://codereview.appspot.com/2183044/
>>
>

[chirag, 2010-09-22 00:44:40]

Yeah, that is correct when we're rendering server-side templates in shindig.

When the document is serialized and presented to the browser, could a
client-side template parser [1][2] need the xmlns attribute? Just
curious about this use-case and am not sure if it's widely used.

[1] - http://code.google.com/p/google-jstemplate/
[2] -
http://svn.apache.org/viewvc/shindig/trunk/features/src/main/javascript/featu...

On Tue, Sep 21, 2010 at 4:42 PM, Mike Samuel <mikesamuel@gmail.com> wrote:
> 2010/9/21  <chiragshah1@gmail.com>:
>> After shindig converts script tags with type=os/* to equivalent OSML
>> tags, it will re-parse each newly converted OSML tag within the correct
>> namespace.
>>
>> Here's the end-to-end flow. 3b starts to explain how xmlns is used:
>> 1) Developer writes the following app:
>> <script type="text/os-template" xmlns:foo="http://example.com"
>> xmlns:os="http://ns.opensocial.org/2008/markup"><foo:Bar
>> attr='Hello'/></script>
>>
>> 2) Shindig parses the document (Neko/Caja) and ends up with a script
>> node containing a text child.
>>
>> 3) Since the script's child is a text node,
>> GadgetHtmlParser#reprocessScriptForOpenSocial() converts the opensocial
>> script tag to an osml tag  and passes the following fragment to Neko for
>> reprocessing.
>> <OSTemplate xmlns:foo="http://example.com"
>> xmlns:os="http://ns.opensocial.org/2008/markup"><foo:Bar
>> attr='Hello'/></OSTemplate>
>>
>> 3b) When the CajaHtmlParser is used the following fragment gets passed
>> for reprocessing:
>> <OSTemplate><foo:Bar attr='Hello'/></OSTemplate>
>>
>> Since xmlns:foo was eliminated,
>> doc.getElementsByTagName("foo:Bar").item(0).getNamespaceURI() returns
>> "unknown://foo" instead of http://example.com.
>>
>> This behavior causes
>> org.apache.shindig.gadgets.templates.tags.DefaultTagRegistry#getHandlerFor()
>> to return null because it needs to find appropriate tag handlers based
>> on the namespaceUri.
>>
>> 4) This newly processed fragment replaces the original script node.
>>
>> This code review in shindig explains this process in more detail:
>> http://codereview.appspot.com/2200046/
>
> Ok, so nothing in that flow requires rendering xmlns declarations.
> Only that they be available as attributes in the DOM so they can be
> migrated to the manufactured top level
>  <os:templates>
> element.
>
> Is that right?
>
>> On 2010/09/21 01:15:31, MikeSamuel wrote:
>>>
>>> 2010/9/20  <chiragshah1@gmail.com>:
>>> > lgtm. I don't have a reference for that specific case, but the
>>> > opensocial-templates spec uses xmlns declarations in script tags.
>>
>>> How are they used?
>>> The only description of xmlns in that doc is talking about xmlns as
>>> used to parse an XML document.  Nothing about why xmlns would need to
>>> be preserved in a document after it's been parsed.
>>
>>> >
>>
>>
http://opensocial-resources.googlecode.com/svn/spec/1.0/OpenSocial-Templating...
>>>
>>> >
>>> >
>>> >
>>> > http://codereview.appspot.com/2183044/
>>
>>
>>
>>
>> http://codereview.appspot.com/2183044/
>>
>

[MikeSamuel, 2010-09-22 05:28:44]

2010/9/21 Chirag Shah <chiragshah1@gmail.com>:
> Yeah, that is correct when we're rendering server-side templates in shindig.

Ok, so getting the part of this change in that preserves xmlns
attributes in the DOM would address that issue.

> When the document is serialized and presented to the browser, could a
> client-side template parser [1][2] need the xmlns attribute? Just

Yep.  That's the question I'm trying to figure out.
And we can delay the render portion of this change until we can answer
that question.

> curious about this use-case and am not sure if it's widely used.
>
> [1] - http://code.google.com/p/google-jstemplate/
> [2] -
http://svn.apache.org/viewvc/shindig/trunk/features/src/main/javascript/featu...
>
> On Tue, Sep 21, 2010 at 4:42 PM, Mike Samuel <mikesamuel@gmail.com> wrote:
>> 2010/9/21  <chiragshah1@gmail.com>:
>>> After shindig converts script tags with type=os/* to equivalent OSML
>>> tags, it will re-parse each newly converted OSML tag within the correct
>>> namespace.
>>>
>>> Here's the end-to-end flow. 3b starts to explain how xmlns is used:
>>> 1) Developer writes the following app:
>>> <script type="text/os-template" xmlns:foo="http://example.com"
>>> xmlns:os="http://ns.opensocial.org/2008/markup"><foo:Bar
>>> attr='Hello'/></script>
>>>
>>> 2) Shindig parses the document (Neko/Caja) and ends up with a script
>>> node containing a text child.
>>>
>>> 3) Since the script's child is a text node,
>>> GadgetHtmlParser#reprocessScriptForOpenSocial() converts the opensocial
>>> script tag to an osml tag  and passes the following fragment to Neko for
>>> reprocessing.
>>> <OSTemplate xmlns:foo="http://example.com"
>>> xmlns:os="http://ns.opensocial.org/2008/markup"><foo:Bar
>>> attr='Hello'/></OSTemplate>
>>>
>>> 3b) When the CajaHtmlParser is used the following fragment gets passed
>>> for reprocessing:
>>> <OSTemplate><foo:Bar attr='Hello'/></OSTemplate>
>>>
>>> Since xmlns:foo was eliminated,
>>> doc.getElementsByTagName("foo:Bar").item(0).getNamespaceURI() returns
>>> "unknown://foo" instead of http://example.com.
>>>
>>> This behavior causes
>>> org.apache.shindig.gadgets.templates.tags.DefaultTagRegistry#getHandlerFor()
>>> to return null because it needs to find appropriate tag handlers based
>>> on the namespaceUri.
>>>
>>> 4) This newly processed fragment replaces the original script node.
>>>
>>> This code review in shindig explains this process in more detail:
>>> http://codereview.appspot.com/2200046/
>>
>> Ok, so nothing in that flow requires rendering xmlns declarations.
>> Only that they be available as attributes in the DOM so they can be
>> migrated to the manufactured top level
>>  <os:templates>
>> element.
>>
>> Is that right?
>>
>>> On 2010/09/21 01:15:31, MikeSamuel wrote:
>>>>
>>>> 2010/9/20  <chiragshah1@gmail.com>:
>>>> > lgtm. I don't have a reference for that specific case, but the
>>>> > opensocial-templates spec uses xmlns declarations in script tags.
>>>
>>>> How are they used?
>>>> The only description of xmlns in that doc is talking about xmlns as
>>>> used to parse an XML document.  Nothing about why xmlns would need to
>>>> be preserved in a document after it's been parsed.
>>>
>>>> >
>>>
>>>
http://opensocial-resources.googlecode.com/svn/spec/1.0/OpenSocial-Templating...
>>>>
>>>> >
>>>> >
>>>> >
>>>> > http://codereview.appspot.com/2183044/
>>>
>>>
>>>
>>>
>>> http://codereview.appspot.com/2183044/
>>>
>>
>

[MikeSamuel, 2010-09-27 19:30:35]

Snapshotted.  Now we preserve in the DOM, but do not render xmlns attributes.

[Jasvir, 2010-09-27 21:19:18]

LGTM

http://codereview.appspot.com/2183044/diff/5002/tests/com/google/caja/parser/...
File tests/com/google/caja/parser/html/NodesTest.java (right):

http://codereview.appspot.com/2183044/diff/5002/tests/com/google/caja/parser/...
tests/com/google/caja/parser/html/NodesTest.java:203: rendered =
Nodes.render(el, true);
/* asXML */ true

http://codereview.appspot.com/2183044/diff/5002/tests/com/google/caja/parser/...
tests/com/google/caja/parser/html/NodesTest.java:228: rendered =
Nodes.render(el, true);
/* asXML */ true

http://codereview.appspot.com/2183044/diff/5002/tests/com/google/caja/parser/...
tests/com/google/caja/parser/html/NodesTest.java:247: return;  // Failure is an
option.
The particular exception in this case is an IllegalStateException thrown by
renderAttr which doesn't seem to be the right exception to throw.

http://codereview.appspot.com/2183044/diff/5002/tests/com/google/caja/plugin/...
File tests/com/google/caja/plugin/stages/RewriteHtmlStageTest.java (right):

http://codereview.appspot.com/2183044/diff/5002/tests/com/google/caja/plugin/...
tests/com/google/caja/plugin/stages/RewriteHtmlStageTest.java:249: "  a ? Hello
<os:template",
It doesn't affect the validity of this test, but this isn't valid OS template
code.

[MikeSamuel, 2010-09-27 22:28:34]

Done, and deprecated the boolean form of Nodes.render since the parameter was
obscure.

http://codereview.appspot.com/2183044/diff/5002/tests/com/google/caja/parser/...
File tests/com/google/caja/parser/html/NodesTest.java (right):

http://codereview.appspot.com/2183044/diff/5002/tests/com/google/caja/parser/...
tests/com/google/caja/parser/html/NodesTest.java:203: rendered =
Nodes.render(el, true);
On 2010/09/27 21:19:18, jasvir wrote:
> /* asXML */ true

true -> MarkupRenderMode.XML

http://codereview.appspot.com/2183044/diff/5002/tests/com/google/caja/parser/...
tests/com/google/caja/parser/html/NodesTest.java:247: return;  // Failure is an
option.
On 2010/09/27 21:19:18, jasvir wrote:
> The particular exception in this case is an IllegalStateException thrown by
> renderAttr which doesn't seem to be the right exception to throw.

Changed to IllegalArgumentException

[Jasvir, 2010-09-29 05:48:23]

LGTM