FYI Looks like this CL missed adding the taming.js files to the pom.xmls -- fixing that now.
Apologies - John 2009/10/29 John Hjelmstad <johnfa...@gmail.com> > Patch committed. I'll expect your JS follow-up after I get in the > FeatureRegistry CL :) > > 2009/10/28 John Hjelmstad <johnfa...@gmail.com> > > 2009/10/28 ๏̯͡๏ Jasvir Nagra <jas...@google.com> >> >> >>> >>> On Tue, Oct 27, 2009 at 6:21 PM, John Hjelmstad <johnfa...@gmail.com>wrote: >>> >>>> Hey Jas: >>>> >>>> As I noted to you recently, I've finally gotten the JS feature loader CL >>>> out. It's here: http://codereview.appspot.com/143046 >>>> >>>> The impact this would have on your CL is that it allows for introduction >>>> of syntax that would include tamings.js only when feature=caja is included >>>> (that, in turn, will require making some kind of gadget processing context >>>> available to rewriters et al). >>>> >>>> The underlying design question I have - not necessarily for this CL - is >>>> whether "feature=caja is included somewhere in the Gadget feature >>>> dependency >>>> tree" will always be equivalent to "Gadget is cajoled". >>>> >>> >>> Yes. If the feature is required implies the content will be cajoled. >>> >> >> Yeah, I was more getting at the reverse here - if cajoled, does the gadget >> require feature=caja? As you note, it does not. >> >> Anyway, all this will affect the design of the Feature loader stuff moreso >> than this CL. I'll patch yours in shortly. >> >> --j >> >> >>> >>> >>>> In particular, will this be true for cajoled-inlined content? I know >>>> we've discussed various ideas around this: <Content type="caja">, <Content >>>> type="html" cajolable="true">, <Require feature="caja">, or simply [ >>>> container chooses whether or not to cajole, no syntax in gadget ]. Thoughts >>>> on this? >>>> >>> >>> Unfortunately this is not true. As it stands a container can externally >>> turn on cajoling but passing a uri parameter flag to turn on cajoling (using >>> &caja=1) and include the caja runtime library (&libs=caja). Both the >>> parameters are needed to run cajoled gadgets correctly and are used by >>> containers. >>> >>> >>>> >>>> In the interim, I don't want to hold you up too much, and feel that >>>> including these tamings should be OK even though it's unnecessary out of >>>> Caja context. Others have an opinion? >>>> >>> >>> I'd really like to see the CL land as it enables correctly use of >>> opensocial and osapi with cajoled gadgets. I'd be keen to get this >>> committed sooner than later - if it really adds undue size to the uncajoled >>> code, I am happy to make the changes required to use the new JsFeatureLoader >>> to only load taming.js if its needed in a separate change. >>> >>> >>>> >>>> --j >>>> >>>> >>>> On Sun, Oct 25, 2009 at 11:18 PM, <jas...@gmail.com> wrote: >>>> >>>>> Snapshot. >>>>> >>>>> >>>>> On 2009/10/21 19:03:23, jasvir wrote: >>>>> >>>>>> http://codereview.appspot.com/135051/diff/1027/48 >>>>>> File features/src/main/javascript/features/caja/taming.js (right): >>>>>> >>>>> >>>>> http://codereview.appspot.com/135051/diff/1027/48#newcode105 >>>>>> Line 105: var tamings___ = tamings___ || []; >>>>>> This works for now. Its vulnerable to a feature you don't trust >>>>>> >>>>> resetting this >>>>> >>>>>> array entirely to prevent it from getting exposed to a gadget but if >>>>>> >>>>> you have a >>>>> >>>>>> feature you don't trust, it can do anything anyways. >>>>>> >>>>> >>>>> On 2009/10/20 21:53:57, johnfargo wrote: >>>>>> > Not that it's a big deal in this case, but maybe it should be. This >>>>>> >>>>> is one of >>>>> >>>>>> a >>>>>> > few use cases I've seen arise that call for a clearer representation >>>>>> >>>>> of the >>>>> >>>>>> > feature dependency tree. >>>>>> >>>>> >>>>> http://codereview.appspot.com/135051/diff/1027/46 >>>>>> File features/src/main/javascript/features/flash/taming.js (right): >>>>>> >>>>> >>>>> http://codereview.appspot.com/135051/diff/1027/46#newcode1 >>>>>> Line 1: /* >>>>>> On 2009/10/20 21:53:57, johnfargo wrote: >>>>>> > Missing a corresponding feature.xml update for flash. >>>>>> >>>>> >>>>> Done. >>>>>> >>>>> >>>>> >>>>> >>>>> http://codereview.appspot.com/135051 >>>>> >>>> >>>> >>> >> >
