> On 2011-05-17 00:56:02, Paul Lindner wrote: > > LGTM -- Tests pass > > > > How risky is the json parsing change? Did you test this out in various > > browsers? > > > > > > felix lee wrote: > it's not very risky, the code path only happens if Caja is enabled, and > prior to this Caja didn't really work. the change to gadgets.json.stringify > makes it use the ES5 definition of JSON.stringify, which everyone has > converged to. > > at the moment, this patch makes Chrome and Safari work for the cajoled > sample gadget I'm testing. Firefox and Opera are still broken, which I'll > fix in a subsequent patch. (they still work fine if I turn off caja.) I > haven't looked at IE yet.
okay, that's good. I'll commit now. - Paul ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/747/#review678 ----------------------------------------------------------- On 2011-05-17 00:20:21, felix lee wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/747/ > ----------------------------------------------------------- > > (Updated 2011-05-17 00:20:21) > > > Review request for shindig. > > > Summary > ------- > > 1. osapi dynamically defined methods are now tamed correctly. > > 2. the browser-native gadgets.json.stringify had a bug in the replacer > function. for keys that should be ignored, it was returning null instead of > undefined, so a value like {a___:3} was being stringified as {'a___':null} > rather than {} > > 3. features/alltests.js didn't work, because it was out of sync with reality. > I updated it to match the jsunit rule in pom.xml. > > > Diffs > ----- > > /trunk/features/src/main/javascript/features/caja/feature.xml 1103952 > /trunk/features/src/main/javascript/features/caja/taming.js 1103952 > /trunk/features/src/main/javascript/features/core.json/json-jsimpl.js > 1103952 > /trunk/features/src/main/javascript/features/core.json/json-native.js > 1103952 > /trunk/features/src/main/javascript/features/opensocial-reference/taming.js > 1103952 > /trunk/features/src/main/javascript/features/osapi.base/feature.xml 1103952 > /trunk/features/src/main/javascript/features/osapi.base/osapi.js 1103952 > /trunk/features/src/main/javascript/features/osapi.base/taming.js 1103952 > /trunk/features/src/test/javascript/features/alltests.js 1103952 > /trunk/features/src/test/javascript/features/mocks/env.js 1103952 > > Diff: https://reviews.apache.org/r/747/diff > > > Testing > ------- > > "mvn test" passes > sample osapi gadget loaded in sample container works (mostly). > > > Thanks, > > felix > >
