https://codereview.appspot.com/10341043/diff/1/src/com/google/caja/ses/repairES5.js
File src/com/google/caja/ses/repairES5.js (right):

https://codereview.appspot.com/10341043/diff/1/src/com/google/caja/ses/repairES5.js#newcode222
src/com/google/caja/ses/repairES5.js:222: sev.level >=
ses.severities.MAGICAL_UNICORN.level &&
On 2013/06/17 17:50:42, kpreid2 wrote:
I propose introducing the following constants next to the definition
of
ses.severities.
var LOWEST_SEVERITY = ses.severities.MAGICAL_UNICORN;
var HIGHEST_SEVERITY = ses.severities.NOT_SUPPORTED;
These mean that future revisions will be more likely to preserve the
correctness
of this code without needing an equivalent of this CL.

I think that's a bad idea because it obscures why the comparison is >=
LOWEST but < HIGHEST. It's deceptively similar to a half-open interval,
which it isn't.

https://codereview.appspot.com/10341043/diff/1/src/com/google/caja/ses/repairES5.js#newcode4039
src/com/google/caja/ses/repairES5.js:4039: if (postSeverity !==
severities.SAFE && disposition(kludge).permit) {
On 2013/06/17 17:50:42, kpreid2 wrote:
On 2013/06/17 17:42:30, MarkM wrote:
> This "!==" should be ">"

Only with .level, that is, postSeverity.level > severities.SAFE.level.

(The test works as is because it is really comparing against the
initialization
of "postSeverity = severities.SAFE;", not an arbitrary severity — but
I agree
this is appropriate in principle.)

Done.

https://codereview.appspot.com/10341043/

--

--- 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.


Reply via email to