Re: OpenSocial 1.0 spec patch: Clarify custom tag parameter behavior

[le...@google.com]

This seems to have a number of changes that were never discussed on the
thread.


http://codereview.appspot.com/151062/diff/1/2
File OpenSocial-Templating.xml (right):

http://codereview.appspot.com/151062/diff/1/2#newcode420
OpenSocial-Templating.xml:420: <t>Parameters can be passed into
templates as XML attributes, elements, or with the &lt;os:Var&gt; tag.
I don't recall the <os:Var> proposal mentioning such usage, and no
examples are given in this patch.

This functionality was never discussed.

http://codereview.appspot.com/151062/diff/1/2#newcode430
OpenSocial-Templating.xml:430:
&lt;messageColor&gt;blue&lt;/messageColor&gt;
Why was this refactored? This example demonstrated a very useful aspect
of ${My} - dropping support for this was never discussed.

http://codereview.appspot.com/151062/diff/1/2#newcode436
OpenSocial-Templating.xml:436: Custom tag template parameters using the
rules below to determine behavior.
This sentence seems to be missing a verb? Meaning is unclear.

http://codereview.appspot.com/151062/diff/1/2#newcode457
OpenSocial-Templating.xml:457: Nested elements are treated as string
content and subject to any escaping rules or behavior.
I believe there is a section detailing how DOM elements are treated in
EL. (
http://www.opensocial.org/Technical-Resources/opensocial-spec-v09/OpenSocial-Templating.html#rfc.section.6.2
)
Why treat them like strings here?

http://codereview.appspot.com/151062/diff/1/2#newcode459
OpenSocial-Templating.xml:459: <preamble>Below code results in:
${My.message} = "This is &lt;b&gt;great&lt;/b&gt;"</preamble>
BTW, in the Wiki version, the <b> tag is not escaped on this line. Just
to be clear, you are saying that it will get escaped, right?

http://codereview.appspot.com/151062/diff/1/2#newcode480
OpenSocial-Templating.xml:480: property of the object.
This "value" property was never discussed - it seems designed to do the
same thing as <os:Render>. (It's possible that a similar scheme can be
used to implement <os:Render>, but I don't think it should be codified
in the spec)

The name "value" is also way too common to be reserved. (In HTML,
<input> and <option> tags use it, suggesting a pattern developers may
chose to replicate for custom tags.)

http://codereview.appspot.com/151062

[goosemanjack]

As I was writing up the patch, it became clear there were a number of
edge cases that needed to be included. The intent is not to sneak
things into the spec, but to insure this patch actually covers all
usages without leaving ambiguity. Comments below.

> I don't recall the <os:Var> proposal mentioning such usage, and no
> examples are given in this patch.

These examples are discussed and shown in the os:Var patch. One of
the main reasons for including os:Var in 1.0 is to improve on local
variables in templates, which overlaps with custom tag parameters.
The patch calls this out to remove ambiguity.
http://wiki.opensocial.org/index.php?title=Os:Var_variables_in_templates_and_data_pipelining#Scope

If need be, they can be repeated in this section or we can add a cross-
reference to usage.

> Why was this refactored? This example demonstrated a very useful aspect
> of ${My} - dropping support for this was never discussed.

The original example was not dropped, but moved to item 4 in the patch
where expected behavior is specifically called out.

Element parameters that contain attributes will result in an
object that has the
attribute values placed as property values of the root object.

Below code results in: ${My.messageStyle.color} = "blue"
<myapp:HelloWorld >
<messageStyle color="blue" />
</myapp:HelloWorld>

> I believe there is a section detailing how DOM elements are treated in

> EL. (http://www.opensocial.org/Technical-Resources/opensocial-spec-v09/Ope...

> )
> Why treat them like strings here?

Treating them like strings here allows for the creation of content
with markup and assigning to keys without the downside of disallowing
other variable types. Content with rudimentary-to-complex markup
appears to be a much more common use-case for parameters than using
them to create complex data structures. Complex data structures are
more easily created with javascript and registered with the data
context using the JS API. Treating nested elements as strings
satisfies the rendered content use-case.

> BTW, in the Wiki version, the <b> tag is not escaped on this line. Just
> to be clear, you are saying that it will get escaped, right?

It will follow EL escaping rules. In this case, if auto-escaping is
enabled this translates to having the <b> become html-encoded to
&lt;b&gt;

> The name "value" is also way too common to be reserved. (In HTML,
> <input> and <option> tags use it, suggesting a pattern developers may
> chose to replicate for custom tags.)

I agree that "value" it is less than ideal. Do you have an alternate
suggestion? The patch could be modified to be "innerXML" or
"innerHTML". Alternately, we could change this to state that if
attributes are included on an element, the content within the tag will
be ignored.

Do you have a preference on this point?
--
clc

On Nov 10, 10:29 am, le...@google.com wrote:
> This seems to have a number of changes that were never discussed on the
> thread.
>
> http://codereview.appspot.com/151062/diff/1/2
> File OpenSocial-Templating.xml (right):
>
> http://codereview.appspot.com/151062/diff/1/2#newcode420
> OpenSocial-Templating.xml:420: <t>Parameters can be passed into
> templates as XML attributes, elements, or with the &lt;os:Var&gt; tag.
> I don't recall the <os:Var> proposal mentioning such usage, and no
> examples are given in this patch.
>
> This functionality was never discussed.
>
> http://codereview.appspot.com/151062/diff/1/2#newcode430
> OpenSocial-Templating.xml:430:
> &lt;messageColor&gt;blue&lt;/messageColor&gt;
> Why was this refactored? This example demonstrated a very useful aspect
> of ${My} - dropping support for this was never discussed.
>
> http://codereview.appspot.com/151062/diff/1/2#newcode436
> OpenSocial-Templating.xml:436: Custom tag template parameters using the
> rules below to determine behavior.
> This sentence seems to be missing a verb? Meaning is unclear.
>
> http://codereview.appspot.com/151062/diff/1/2#newcode457
> OpenSocial-Templating.xml:457: Nested elements are treated as string
> content and subject to any escaping rules or behavior.
> I believe there is a section detailing how DOM elements are treated in

> EL. (http://www.opensocial.org/Technical-Resources/opensocial-spec-v09/Ope...

[Lev Epshteyn]

On Tue, Nov 10, 2009 at 7:50 PM, goosemanjack <cc...@myspace.com> wrote:

As I was writing up the patch, it became clear there were a number of
edge cases that needed to be included.  The intent is not to sneak
things into the spec, but to insure this patch actually covers all
usages without leaving ambiguity.  Comments below.

> I don't recall the <os:Var> proposal mentioning such usage, and no
> examples are given in this patch.

These examples are discussed and shown in the os:Var patch.  One of
the main reasons for including os:Var in 1.0 is to improve on local
variables in templates, which overlaps with custom tag parameters.
The patch calls this out to remove ambiguity.
http://wiki.opensocial.org/index.php?title=Os:Var_variables_in_templates_and_data_pipelining#Scope

If need be, they can be repeated in this section or we can add a cross-
reference to usage.

Sorry, I missed this use case in the <os:Var> proposal. I'm still not sure how useful such constructs would be - I think I asked on the <os:Var> thread for examples of some real world use cases of how it was to be used.

There are currently two ways of passing params to custom tags - attributes and child nodes. Adding a third one is something we should only do if it enables significantly useful applications. <os:Var> in this context would be useful if it allowed us to define non-hardcoded complex data objects. (The existing methods allow passing scalars, arrays of scalars and references to existing data.) I still don't know how <os:Var> can be used to achieve this as proposed, so I don't see any new functionality enabled by allowing it to be used in this fashion. Though I guess this is more of a feedback to the <os:Var> proposal.
 

> Why was this refactored? This example demonstrated a very useful aspect
> of ${My} - dropping support for this was never discussed.

The original example was not dropped, but moved to item 4 in the patch
where expected behavior is specifically called out.

Ah. OK, my bad. I'm all for explicitly calling out this functionality.

   Element parameters that contain attributes will result in an
object that has the
    attribute values placed as property values of the root object.

    Below code results in: ${My.messageStyle.color} = "blue"
      <myapp:HelloWorld >
         <messageStyle color="blue" />
       </myapp:HelloWorld>

> I believe there is a section detailing how DOM elements are treated in

> EL. (http://www.opensocial.org/Technical-Resources/opensocial-spec-v09/Ope...

> )
> Why treat them like strings here?

Treating them like strings here allows for the creation of content
with markup and assigning to keys without the downside of disallowing
other variable types.  Content with rudimentary-to-complex markup
appears to be a much more common use-case for parameters than using
them to create complex data structures.  Complex data structures are
more easily created with javascript and registered with the data
context using the JS API.  Treating nested elements as strings
satisfies the rendered content use-case.

This is why we have <os:Render> support - it recognizes that when you have markup inside custom tags, it must be treated well. This is why <os:Render> will not escape the markup (since it's markup coming from markup, not markup coming from user data) and will explicitly support any templating directives in the markup.

I am opposed to adding another way of rendering content by changing how DOM nodes work with EL in this "special" case. I don't see any reason why you'd want to refer to the content markup other than to render it, and rendering is something we can already do with <os:Render>
 

> BTW, in the Wiki version, the <b> tag is not escaped on this line. Just
> to be clear, you are saying that it will get escaped, right?

It will follow EL escaping rules.  In this case, if auto-escaping is
enabled this translates to having the <b> become html-encoded to
&lt;b&gt;

OK. There's just an inconsistency between the Wiki version and the code review version.
 

> The name "value" is also way too common to be reserved. (In HTML,
> <input> and <option> tags use it, suggesting a pattern developers may
> chose to replicate for custom tags.)

I agree that "value" it is less than ideal.  Do you have an alternate
suggestion?  The patch could be modified to be "innerXML" or
"innerHTML".  Alternately, we could change this to state that if
attributes are included on an element, the content within the tag will
be ignored.

Do you have a preference on this point?

I definitely like "inner??L" better than "value", but again, I'm not sure I see the need for the feature in the first place, since it's duplicating <os:Render>. Having content markup mixed in with data markup is not a good idea, and we shouldn't encourage people to do it. At the very least, I'd expect the user to put data and content into separate child elements, in which case the "content" part is addressable individually.
 

--

You received this message because you are subscribed to the Google Groups "OpenSocial and Gadgets Specification Discussion" group.
To post to this group, send email to opensocial-an...@googlegroups.com.
To unsubscribe from this group, send email to opensocial-and-gadg...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/opensocial-and-gadgets-spec?hl=.

[Evan Gilbert]

One note below.

It would be useful to have use cases that we could show a before/after for to help understand why we need some of these changes.

I agree with Lev's comments. The existing syntax supports rendering DOM content with <os:Render> and supports common use cases with more complicated child structures, such as:

<my:TabView>

  <my:Tab selected="${expr}">

    <my:Title>....</my:Title>

    <my:Body style="funky">...</my:Body>

  </my:Tab>

  <my:Tab selected="${otherexpr}">

    <my:Title>....</my:Title>

    <my:Body style="notfunky">...</my:Body>

  </my:Tab>

</my:TabView>

[goosemanjack]

The two weaknesses of os:Render are that

1. The intent of the parameters is not clear - the presence of
os:Render has a side effect of them being converted from data to text
(markup)

2. The presence of os:Render precludes having data parameters.

If you strongly disagree with treating nested nodes as text I'm OK
with striking this section from the patch. You can rely on the EL
text about how to treat DOM nodes to specify this behavior.

On Nov 10, 10:55 pm, Evan Gilbert <uid...@google.com> wrote:
> One note below.
>
> It would be useful to have use cases that we could show a before/after for
> to help understand why we need some of these changes.
>
>
>
> On Tue, Nov 10, 2009 at 5:15 PM, Lev Epshteyn <le...@google.com> wrote:
>
> > On Tue, Nov 10, 2009 at 7:50 PM, goosemanjack <cc...@myspace.com> wrote:
>
> >> As I was writing up the patch, it became clear there were a number of
> >> edge cases that needed to be included.  The intent is not to sneak
> >> things into the spec, but to insure this patch actually covers all
> >> usages without leaving ambiguity.  Comments below.
>
> >> > I don't recall the <os:Var> proposal mentioning such usage, and no
> >> > examples are given in this patch.
>
> >> These examples are discussed and shown in the os:Var patch.  One of
> >> the main reasons for including os:Var in 1.0 is to improve on local
> >> variables in templates, which overlaps with custom tag parameters.
> >> The patch calls this out to remove ambiguity.
>

> >>http://wiki.opensocial.org/index.php?title=Os:Var_variables_in_templa...

> >> opensocial-and-gadg...@googlegroups.com<opensocial-and-gadgets-spec%2Bunsu...@googlegroups.com>
> >> .

> >> For more options, visit this group at
> >>http://groups.google.com/group/opensocial-and-gadgets-spec?hl=.
>
> >  --
> > You received this message because you are subscribed to the Google Groups
> > "OpenSocial and Gadgets Specification Discussion" group.
> > To post to this group, send email to
> > opensocial-an...@googlegroups.com.
> > To unsubscribe from this group, send email to

> > opensocial-and-gadg...@googlegroups.com<opensocial-and-gadgets-spec%2Bunsu...@googlegroups.com>
> > .

[Lev Epshteyn]

On Thu, Nov 12, 2009 at 5:38 PM, goosemanjack <cc...@myspace.com> wrote:

The two weaknesses of os:Render are that

1. The intent of the parameters is not clear - the presence of
os:Render has a side effect of them being converted from data to text
(markup)

Well, that is the intent. It enables formatting tags to be created. The usage of <os:Render> is what tells the processor that child nodes are content. I really don't see how we can implement something like this without it:

<my:SupaBold>[Arbitrarily complex markup w/ templating here]</my:SupaBold>

 

2. The presence of os:Render precludes having data parameters.

I don't see how that is either

<script tag="my:SupaBold">
  <div class="supa-bold" style="color: ${My.color}"><os:Render/></div>
</script>
<my:SupaBold color="green">[Arbitrarily complex markup w/ templating here]</my:SupaBold>
 

There doesn't seem to be any ambiguity here.

In any case, since we are not discussing the removal of <os:Render> from the spec, I don't see why we should consider adding something that accomplishes a subset of the same functionality.

[sspweb...@gmail.com]

http://codereview.appspot.com/151062

[goosemanjack]

> In any case, since we are not discussing the removal of <os:Render> from the
> spec, I don't see why we should consider adding something
> that accomplishes a subset of the same functionality.

We actually are discussing deprecating <os:Render> on the below
thread. Add any additional comments on that thread.

http://groups.google.com/group/opensocial-and-gadgets-spec/browse_thread/thread/309e332e5f46af7c/aa6f40d00f10825d?hl=en#aa6f40d00f10825d
--
clc

[goosemanjack]

It looks like the only remaining contentious issue is treatment of
markup in parameters as string values:

(Item #3 from patch)
> Custom tags only register element parameters that are direct child nodes
> of the custom tag instance. Nested elements are treated as string content

> and subject to any escaping rules or behavior.

If we strike this from the proposal, is everything else good to go?
--
clc

On Dec 15 2009, 9:35 am, goosemanjack <cc...@myspace.com> wrote:
> > In any case, since we are not discussing the removal of <os:Render> from the
> > spec, I don't see why we should consider adding something
> > that accomplishes a subset of the same functionality.
>
> We actually are discussing deprecating <os:Render> on the below
> thread.  Add any additional comments on that thread.
>

> http://groups.google.com/group/opensocial-and-gadgets-spec/browse_thr...

[Lev Epshteyn]

I think this brings us much closer. I'd also like to see the mention of usage with <os:Var> and innerXML dropped, since I think we decided to make the spec be silent on these (on the other thread).

Chris, can you upload an updated proposal with these changes, so we can vote on it?

--
You received this message because you are subscribed to the Google Groups "OpenSocial and Gadgets Specification Discussion" group.
To post to this group, send email to opensocial-an...@googlegroups.com.

To unsubscribe from this group, send email to opensocial-and-gadg...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/opensocial-and-gadgets-spec?hl=en.

[goosemanjack]

I'll get that uploaded shortly.

> > opensocial-and-gadg...@googlegroups.com<opensocial-and-gadgets-spec%2Bunsu...@googlegroups.com>

[goosemanjack]

Updated patch has been added. Items referencing "innerXML" and using
markup in parameters have been removed.

[Evan Gilbert]

This patch looks great,

To unsubscribe from this group, send email to opensocial-and-gadg...@googlegroups.com.

[Evan Gilbert]

sorry, early send

This patch looks great. +1 overall, think there's one reference to <os:Var> at the top that probably should be taken out.

Evan

[goosemanjack]

Removed reference to os:Var and applied patch for RC1 publishing.
--
clc

> >> <opensocial-and-gadgets-spec%2Bunsu...@googlegroups.com<opensocial-and-gadgets-spec%252Buns...@googlegroups.com>

[Lev Epshteyn]

+1

To unsubscribe from this group, send email to opensocial-and-gadg...@googlegroups.com.

[Jalen]

+1

[jorge]

+1