https://codereview.appspot.com/7678045/diff/12001/build.xml
File build.xml (right):
https://codereview.appspot.com/7678045/diff/12001/build.xml#newcode422
build.xml:422: <succeed message=
On 2013/04/03 07:13:49, Jasvir wrote:
Debugging?
Done.
https://codereview.appspot.com/7678045/diff/12001/src/com/google/caja/parser/js/PlainModule.java
File src/com/google/caja/parser/js/PlainModule.java (right):
https://codereview.appspot.com/7678045/diff/12001/src/com/google/caja/parser/js/PlainModule.java#newcode57
src/com/google/caja/parser/js/PlainModule.java:57: public Object
getValue() {
On 2013/04/03 07:13:49, Jasvir wrote:
Remind me how this value is used?
It should not be. It's just the inherited member accessor for things
like the "value" of a ParseTreeNode representing a literal, etc.
https://codereview.appspot.com/7678045/diff/12001/src/com/google/caja/parser/quasiliteral/TracingRewriter.java
File src/com/google/caja/parser/quasiliteral/TracingRewriter.java
(right):
https://codereview.appspot.com/7678045/diff/12001/src/com/google/caja/parser/quasiliteral/TracingRewriter.java#newcode270
src/com/google/caja/parser/quasiliteral/TracingRewriter.java:270:
"fname", bindings.get("fname"),
On 2013/04/03 07:13:49, Jasvir wrote:
Does this do the right thing when rendering and @fname was null?
The test cases have a lot of anon functions in them and they work, so
yes.
https://codereview.appspot.com/7678045/diff/12001/src/com/google/caja/parser/quasiliteral/TracingRewriter.java#newcode294
src/com/google/caja/parser/quasiliteral/TracingRewriter.java:294:
substitutes="TRACING.log(@lit + @expr)")
On 2013/04/03 07:13:49, Jasvir wrote:
Nice!
Done.
https://codereview.appspot.com/7678045/diff/12001/src/com/google/caja/parser/quasiliteral/TracingRewriter.java#newcode312
src/com/google/caja/parser/quasiliteral/TracingRewriter.java:312:
matches="new @ctor(@args*)",
On 2013/04/03 07:13:49, Jasvir wrote:
Do you need to also handle new complex expr - like new x.y?
Yes, that should work fine given that @ctor is expand()-ed below.
https://codereview.appspot.com/7678045/diff/12001/src/com/google/caja/parser/quasiliteral/TracingRewriter.java#newcode422
src/com/google/caja/parser/quasiliteral/TracingRewriter.java:422: + "
TRACING.@pushCallsite('function', @pos, @name);"
On 2013/04/03 07:13:49, Jasvir wrote:
Check that pushCallsite and popCallsite don't throw their own
exceptions.
Good point. My hope is that these 2 functions are simple enough that
they are too small to fail. Or are things too big to fail these days? I
don't know. Anyway.
https://codereview.appspot.com/7678045/diff/12001/src/com/google/caja/plugin/TracingRewriterMain.java
File src/com/google/caja/plugin/TracingRewriterMain.java (right):
https://codereview.appspot.com/7678045/diff/12001/src/com/google/caja/plugin/TracingRewriterMain.java#newcode75
src/com/google/caja/plugin/TracingRewriterMain.java:75:
dumpMessages(mq);
On 2013/04/03 07:13:49, Jasvir wrote:
dumpMessages(mq);
return;
...would save you the levels of nesting.
Done.
https://codereview.appspot.com/7678045/diff/12001/src/com/google/caja/service/HtmlHandler.java
File src/com/google/caja/service/HtmlHandler.java (right):
https://codereview.appspot.com/7678045/diff/12001/src/com/google/caja/service/HtmlHandler.java#newcode102
src/com/google/caja/service/HtmlHandler.java:102: boolean htmlInline,
Writer output,
On 2013/04/03 07:13:49, Jasvir wrote:
Any reason to restrict this to a Writer?
Yeah -- because the new-and-improved renderAsJSON would not accept it.
Note this is just a private method, not part of the inherited API of the
class.
https://codereview.appspot.com/7678045/diff/12001/src/com/google/caja/tracing.js
File src/com/google/caja/tracing.js (right):
https://codereview.appspot.com/7678045/diff/12001/src/com/google/caja/tracing.js#newcode88
src/com/google/caja/tracing.js:88: ctx = ctx.parent;
The tracer always (or is supposed to, any rate) write out programs with
a symmetrical number of pushes and pops. Your hypothetical program is
not legal JavaScript, but for other similar things, the tracer writes
out a push and a pop for the program as a whole.
https://codereview.appspot.com/7678045/diff/12001/src/com/google/caja/tracing.js#newcode101
src/com/google/caja/tracing.js:101: if (!ctx.log) {
On 2013/04/03 07:13:49, Jasvir wrote:
Ah this is not initialized along with the rest of ctx in clear()
because it's
supposed to last across clear()s.
Done.
https://codereview.appspot.com/7678045/diff/12001/src/com/google/caja/tracing.js#newcode108
src/com/google/caja/tracing.js:108: var xhr = new XMLHttpRequest();
On 2013/04/03 07:13:49, Jasvir wrote:
Consider doing the check for XDomainRequest since this tool would
probably come
in really handy on IE where xhr isn't CORS enabled.
Noted as an enhancement, thank you. The code has lain fallow for too
long so I'm trying to get it checked in 1st.
https://codereview.appspot.com/7678045/
--
---
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.
