Patch committed. I'll expect your JS follow-up after I get in the FeatureRegistry CL :)
2009/10/28 John Hjelmstad <[email protected]> > 2009/10/28 ๏̯͡๏ Jasvir Nagra <[email protected]> > > >> >> On Tue, Oct 27, 2009 at 6:21 PM, John Hjelmstad <[email protected]>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, <[email protected]> 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 >>>> >>> >>> >> >
