http://codereview.appspot.com/50041/diff/10037/8109 File src/com/google/caja/cajita-debugmode.js (left): http://codereview.appspot.com/50041/diff/10037/8109#oldcode81 Line 81: On 2009/07/06 21:11:08, ihab.awad wrote:
Why can we delete this stuff? Doesn't it do anything useful any more?
No. It was only used by the deleted code below, where this is explained.
Do the debug mode tests work without it?
Yes. http://codereview.appspot.com/50041/diff/10037/8109#oldcode81 Line 81: On 2009/07/06 21:11:08, ihab.awad wrote:
Why can we delete this stuff? Doesn't it do anything useful any more?
Do the
debug mode tests work without it?
Done. http://codereview.appspot.com/50041/diff/10037/8109#oldcode359 Line 359: ], 0); On 2009/07/06 21:11:08, ihab.awad wrote:
Same question re deleting stuff.
The purpose of this was to suppress the setting of fastpath flags, so that, for example, calls would always go through callPub(), which is overriden in debugmode with extra bookkeeping. However, debug mode also rewrites the translation so that the fastpath flags aren't checked by the cajoler output anyway. Therefore, the above short circuiting isn't needed. As issue 1058 explains, this short circuiting didn't actually work, so we're lucky we can simply delete it. I wish they were all this easy ;). http://codereview.appspot.com/50041/diff/10037/8109#oldcode359 Line 359: ], 0); On 2009/07/06 21:11:08, ihab.awad wrote:
Same question re deleting stuff.
Done. http://codereview.appspot.com/50041/diff/10037/8111 File src/com/google/caja/cajita.js (right): http://codereview.appspot.com/50041/diff/10037/8111#newcode465 Line 465: fail('Not writable: (', debugReference(obj), ').', name); On 2009/07/06 10:26:20, ben wrote:
If we're going to call it writeable, should this be handleWrite? Feel
free to
just add a FIXME rather than actually make this change.
New text near the top of cajita.js: // FIXME(erights): After the Caja runtime was started, some of Caja's // ideas for controlling ES3 attribute helped inspire the new ES5 // attribute control APIs. But in the process, different terms were // adopted for these attributes. The current correspondences: // ES3 Caja ES5 // ReadOnly canSet writable // DontEnum canEnum enumerable // DontDelete canDelete configurable // We should consider migrating our terminology towards ES5's, // especially changing "set" to "write". http://codereview.appspot.com/50041/diff/10037/8111#newcode465 Line 465: fail('Not writable: (', debugReference(obj), ').', name); On 2009/07/06 10:26:20, ben wrote:
If we're going to call it writeable, should this be handleWrite? Feel
free to
just add a FIXME rather than actually make this change.
Done. http://codereview.appspot.com/50041/diff/10037/8111#newcode1073 Line 1073: * ___.ctor(). On 2009/07/06 10:26:20, ben wrote:
It worries me that, AFAICS, we don't (can't?) check that someSuper actually is a superclass,
It is not that someSuper already was a superclass. As it says earlier above, "extend() also sets hiddenCtor.prototype to set up the prototype chain so that [..]". In other words, extend() initializes hiddenCtor so that it represents a "subclass" of someSuper. I have added text to this effect at the beginning of the doc-comment.
or that it meets these constraints.
Actually, the code "someSuper = asCtor(someSuper.prototype.constructor);" below already checks exactly this condition. http://codereview.appspot.com/50041/diff/10037/8111#newcode1073 Line 1073: * ___.ctor(). On 2009/07/06 10:26:20, ben wrote:
It worries me that, AFAICS, we don't (can't?) check that someSuper
actually is a
superclass, or that it meets these constraints.
Done. http://codereview.appspot.com/50041/diff/10037/8116 File src/com/google/caja/demos/calendar/demo-cajoled.html (right): http://codereview.appspot.com/50041/diff/10037/8116#newcode94 Line 94: getImports: ___.frozenFunc(function () { On 2009/07/06 10:26:20, ben wrote:
NiceNeighbor does not explain (to me) the difference between
___.frozenFunc()
and ___.grantFunc(). If the only difference is that one takes a
function and the
other takes obj, name, then why such different names?
Also, should it not be ___.freezeFunc()?
Fixed according to our verbal agreement: frozenFunc -> markFuncFreeze ctor -> markCtor func -> markFuncOnly (only for use by cajoler output) xo4a -> markXo4a (only inside cajita.js) http://codereview.appspot.com/50041/diff/10037/8116#newcode94 Line 94: getImports: ___.frozenFunc(function () { On 2009/07/06 10:26:20, ben wrote:
NiceNeighbor does not explain (to me) the difference between
___.frozenFunc()
and ___.grantFunc(). If the only difference is that one takes a
function and the
other takes obj, name, then why such different names?
Also, should it not be ___.freezeFunc()?
Done. http://codereview.appspot.com/50041/diff/10037/8107 File src/com/google/caja/plugin/domita.js (right): http://codereview.appspot.com/50041/diff/10037/8107#newcode3243 Line 3243: // called by cajoled code, so we do not use classUtils.inertClassCtor(). On 2009/07/06 21:11:08, ihab.awad wrote:
s/classUtils.inertClassCtor/inertCtor/
Done. http://codereview.appspot.com/50041
