LGTM 2013/7/12 <[email protected]>: > > https://codereview.appspot.com/11231043/diff/1/src/com/google/caja/plugin/domado.js > File src/com/google/caja/plugin/domado.js (right): > > https://codereview.appspot.com/11231043/diff/1/src/com/google/caja/plugin/domado.js#newcode1950 > src/com/google/caja/plugin/domado.js:1950: "CSS_PROP": prop > Oh, a note of caution: sanitizeStyleProperty used to close over its > cssPropertyName parameter when invoking uriRewrite (line 1965 in base), > but now it uses the one coming back from sanitizeCssProperty (this line > here). I looked at sanitizeCssProperty and I think those should be > identical, but I'd appreciate a confirmation that that's a valid > refactoring. > > > https://codereview.appspot.com/11231043/diff/1/src/com/google/caja/plugin/domado.js#newcode6556 > src/com/google/caja/plugin/domado.js:6556: > allCssProperties.forEachCanonical(function(stylePropertyName) { > On 2013/07/13 00:19:32, MikeSamuel wrote: >> >> Is loop is now including non-canon properties? > > > I don't understand the question. I also don't really understand this > code (and in particular I haven't looked into what a non-canonical > property is), but I attempted to preserve the existing behavior exactly. > As far as I saw based on reading CssPropertiesCollection, > forEachCanonical will return exactly the same set of properties as the > previous isCanonicalProp test allowed. So, this does not add any more > properties to the exposed set. > > So, if there's a bug here (are there supposed to be accessors for > non-canonical properties?) then (a) it's existed since the > Domita-to-Domado transition and (b) it wasn't caught by the tests (I > already ran tests on this change). > > If there is a bug, please file an issue for it and I'll look into it as > a separate change (to keep this a pure refactoring). > > https://codereview.appspot.com/11231043/
-- --- You received this message because you are subscribed to the Google Groups "Google Caja Discuss" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/groups/opt_out.
