Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(294)

Issue 6827077: Mitigate top-level vars and typeof differences between ES5 and ES5/3 with rewriting (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 5 months ago by Jasvir
Modified:
1 year, 2 months ago
Reviewers:
kpreid2, MarkM, metaweta
CC:
google-caja-discuss_googlegroups.com
Base URL:
http://google-caja.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Uses the acorn parser and escodegen serializer to rewrite:
* top level var decls to assignments on the global object
* typeof of undeclared variables to return the string "undefined"

Similarly fixing top-level function declarations will be handled in a separate
CL.

Patch Set 1 #

Total comments: 22

Patch Set 2 : Mitigate top-level vars and typeof differences between ES5 and ES5/3 with rewriting #

Patch Set 3 : Mitigate top-level vars and typeof differences between ES5 and ES5/3 with rewriting #

Patch Set 4 : Mitigate top-level vars and typeof differences between ES5 and ES5/3 with rewriting #

Patch Set 5 : Mitigate top-level vars and typeof differences between ES5 and ES5/3 with rewriting #

Total comments: 10

Patch Set 6 : Mitigate top-level vars and typeof differences between ES5 and ES5/3 with rewriting #

Patch Set 7 : Mitigate top-level vars and typeof differences between ES5 and ES5/3 with rewriting #

Patch Set 8 : Mitigate top-level vars and typeof differences between ES5 and ES5/3 with rewriting #

Total comments: 12

Patch Set 9 : Mitigate top-level vars and typeof differences between ES5 and ES5/3 with rewriting #

Total comments: 4

Patch Set 10 : Mitigate top-level vars and typeof differences between ES5 and ES5/3 with rewriting #

Patch Set 11 : Mitigate top-level vars and typeof differences between ES5 and ES5/3 with rewriting #

Total comments: 2

Patch Set 12 : Mitigate top-level vars and typeof differences between ES5 and ES5/3 with rewriting #

Total comments: 15

Patch Set 13 : Mitigate top-level vars and typeof differences between ES5 and ES5/3 with rewriting #

Total comments: 2

Patch Set 14 : Mitigate top-level vars and typeof differences between ES5 and ES5/3 with rewriting #

Patch Set 15 : Mitigate top-level vars and typeof differences between ES5 and ES5/3 with rewriting #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+4025 lines, -56 lines) Patch
M build.xml View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +22 lines, -0 lines 0 comments Download
M src/com/google/caja/parser/quasiliteral/ES53Rewriter.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +0 lines, -12 lines 0 comments Download
M src/com/google/caja/parser/quasiliteral/RewriterMessageType.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +0 lines, -5 lines 0 comments Download
A src/com/google/caja/ses/createExports.js View 1 2 3 1 chunk +23 lines, -0 lines 0 comments Download
A src/com/google/caja/ses/exportsToSES.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +35 lines, -0 lines 1 comment Download
M src/com/google/caja/ses/hookupSES.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -0 lines 0 comments Download
M src/com/google/caja/ses/hookupSESPlus.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -0 lines 0 comments Download
A src/com/google/caja/ses/mitigateGotchas.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +178 lines, -0 lines 0 comments Download
M src/com/google/caja/ses/startSES.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 6 chunks +22 lines, -3 lines 0 comments Download
M tests/com/google/caja/parser/quasiliteral/ES53RewriterTest.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +0 lines, -6 lines 0 comments Download
M tests/com/google/caja/plugin/Es5BrowserTest.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +4 lines, -0 lines 0 comments Download
M tests/com/google/caja/plugin/es53-test-assert-es53mode.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -11 lines 0 comments Download
M tests/com/google/caja/plugin/es53-test-assert-es5mode.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -11 lines 0 comments Download
M tests/com/google/caja/plugin/es53-test-automode1.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +8 lines, -2 lines 0 comments Download
M tests/com/google/caja/plugin/es53-test-automode2.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +6 lines, -1 line 0 comments Download
M tests/com/google/caja/plugin/es53-test-domado-dom-guest.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -4 lines 0 comments Download
M tests/com/google/caja/plugin/es53-test-inline-script.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -1 line 0 comments Download
M tests/com/google/caja/plugin/es53-test-language-guest.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +49 lines, -0 lines 0 comments Download
A third_party/js/acorn/acorn.js View 1 chunk +1595 lines, -0 lines 0 comments Download
A third_party/js/escodegen/escodegen.js View 1 chunk +1834 lines, -0 lines 0 comments Download
A third_party/js/escodegen/estraverse.js View 1 chunk +243 lines, -0 lines 0 comments Download

Messages

Total messages: 33
Jasvir
1 year, 5 months ago #1
MarkM
https://codereview.appspot.com/6827077/diff/1/src/com/google/caja/ses/startSES.js File src/com/google/caja/ses/startSES.js (right): https://codereview.appspot.com/6827077/diff/1/src/com/google/caja/ses/startSES.js#newcode646 src/com/google/caja/ses/startSES.js:646: exprSrc = mitigateGotchas(exprSrc); Here, you're mitigating gotchas on exprSrc ...
1 year, 5 months ago #2
MarkM
https://codereview.appspot.com/6827077/diff/1/src/com/google/caja/ses/startSES.js File src/com/google/caja/ses/startSES.js (right): https://codereview.appspot.com/6827077/diff/1/src/com/google/caja/ses/startSES.js#newcode633 src/com/google/caja/ses/startSES.js:633: * analogous {@code compileProgram} function that accepts a Now ...
1 year, 5 months ago #3
MarkM
https://codereview.appspot.com/6827077/diff/1/src/com/google/caja/ses/mitigateGotchas.js File src/com/google/caja/ses/mitigateGotchas.js (right): https://codereview.appspot.com/6827077/diff/1/src/com/google/caja/ses/mitigateGotchas.js#newcode18 src/com/google/caja/ses/mitigateGotchas.js:18: * outside the TCB. Please put in a "see" ...
1 year, 5 months ago #4
kpreid2
https://codereview.appspot.com/6827077/diff/1/build.xml File build.xml (right): https://codereview.appspot.com/6827077/diff/1/build.xml#newcode990 build.xml:990: <input file="${third_party}/js/escodegen/escodegen.js" jslint="false"/> Since these third-party files support the ...
1 year, 5 months ago #5
metaweta
https://codereview.appspot.com/6827077/diff/1/src/com/google/caja/ses/mitigateGotchas.js File src/com/google/caja/ses/mitigateGotchas.js (right): https://codereview.appspot.com/6827077/diff/1/src/com/google/caja/ses/mitigateGotchas.js#newcode20 src/com/google/caja/ses/mitigateGotchas.js:20: * Note the parse tree manipulate in this file ...
1 year, 5 months ago #6
Jasvir
https://codereview.appspot.com/6827077/diff/1/build.xml File build.xml (right): https://codereview.appspot.com/6827077/diff/1/build.xml#newcode990 build.xml:990: <input file="${third_party}/js/escodegen/escodegen.js" jslint="false"/> On 2012/11/12 19:04:24, kpreid2 wrote: > ...
1 year, 4 months ago #7
MarkM
The stuff under src/com/google/caja/ses/* LGTM. Due to time limitations I abstain on the rest.
1 year, 4 months ago #8
kpreid2
https://codereview.appspot.com/6827077/diff/18001/src/com/google/caja/ses/exportsToSES.js File src/com/google/caja/ses/exportsToSES.js (right): https://codereview.appspot.com/6827077/diff/18001/src/com/google/caja/ses/exportsToSES.js#newcode26 src/com/google/caja/ses/exportsToSES.js:26: ses.generate = exports.generate; Maybe stuff these on a sub-object ...
1 year, 4 months ago #9
kpreid2
We should also get rid of the es53 warnings TOP_LEVEL_VAR_INCOMPATIBLE_WITH_CAJA TOP_LEVEL_FUNC_INCOMPATIBLE_WITH_CAJA since they are about ...
1 year, 4 months ago #10
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
1 year, 4 months ago #11
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
1 year, 3 months ago #12
Jasvir
snapshot https://codereview.appspot.com/6827077/diff/18001/src/com/google/caja/ses/exportsToSES.js File src/com/google/caja/ses/exportsToSES.js (right): https://codereview.appspot.com/6827077/diff/18001/src/com/google/caja/ses/exportsToSES.js#newcode26 src/com/google/caja/ses/exportsToSES.js:26: ses.generate = exports.generate; On 2012/11/19 19:06:01, kpreid2 wrote: ...
1 year, 3 months ago #13
kpreid2
https://codereview.appspot.com/6827077/diff/29022/src/com/google/caja/parser/quasiliteral/RewriterMessageType.java File src/com/google/caja/parser/quasiliteral/RewriterMessageType.java (right): https://codereview.appspot.com/6827077/diff/29022/src/com/google/caja/parser/quasiliteral/RewriterMessageType.java#newcode191 src/com/google/caja/parser/quasiliteral/RewriterMessageType.java:191: TOP_LEVEL_FUNC_INCOMPATIBLE_WITH_CAJA( Mitigator should be making this obsolete too. https://codereview.appspot.com/6827077/diff/29022/src/com/google/caja/ses/exportsToSES.js ...
1 year, 3 months ago #14
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
1 year, 3 months ago #15
Jasvir
snapshot https://codereview.appspot.com/6827077/diff/29022/src/com/google/caja/parser/quasiliteral/RewriterMessageType.java File src/com/google/caja/parser/quasiliteral/RewriterMessageType.java (right): https://codereview.appspot.com/6827077/diff/29022/src/com/google/caja/parser/quasiliteral/RewriterMessageType.java#newcode191 src/com/google/caja/parser/quasiliteral/RewriterMessageType.java:191: TOP_LEVEL_FUNC_INCOMPATIBLE_WITH_CAJA( In a separate CL. https://codereview.appspot.com/6827077/diff/29022/src/com/google/caja/ses/exportsToSES.js File src/com/google/caja/ses/exportsToSES.js ...
1 year, 3 months ago #16
MarkM
https://codereview.appspot.com/6827077/diff/37001/src/com/google/caja/ses/startSES.js File src/com/google/caja/ses/startSES.js (right): https://codereview.appspot.com/6827077/diff/37001/src/com/google/caja/ses/startSES.js#newcode211 src/com/google/caja/ses/startSES.js:211: mitigateGotchas, Please make this into opt_mitigateGotchas and (early enough) ...
1 year, 3 months ago #17
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
1 year, 3 months ago #18
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
1 year, 3 months ago #19
Jasvir
https://codereview.appspot.com/6827077/diff/37001/src/com/google/caja/ses/startSES.js File src/com/google/caja/ses/startSES.js (right): https://codereview.appspot.com/6827077/diff/37001/src/com/google/caja/ses/startSES.js#newcode211 src/com/google/caja/ses/startSES.js:211: mitigateGotchas, On 2012/12/29 20:27:26, MarkM wrote: > Please make ...
1 year, 3 months ago #20
MarkM
https://codereview.appspot.com/6827077/diff/37001/src/com/google/caja/ses/exportsToSES.js File src/com/google/caja/ses/exportsToSES.js (right): https://codereview.appspot.com/6827077/diff/37001/src/com/google/caja/ses/exportsToSES.js#newcode33 src/com/google/caja/ses/exportsToSES.js:33: delete window.exports; At top level, the platform independent way ...
1 year, 3 months ago #21
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
1 year, 3 months ago #22
Jasvir
https://codereview.appspot.com/6827077/diff/37001/src/com/google/caja/ses/exportsToSES.js File src/com/google/caja/ses/exportsToSES.js (right): https://codereview.appspot.com/6827077/diff/37001/src/com/google/caja/ses/exportsToSES.js#newcode33 src/com/google/caja/ses/exportsToSES.js:33: delete window.exports; Done. The linter was giving me problems ...
1 year, 3 months ago #23
MarkM
caja/ses/* LGTM. As usual, I leave the rest to others.
1 year, 3 months ago #24
kpreid2
https://codereview.appspot.com/6827077/diff/29022/src/com/google/caja/parser/quasiliteral/RewriterMessageType.java File src/com/google/caja/parser/quasiliteral/RewriterMessageType.java (right): https://codereview.appspot.com/6827077/diff/29022/src/com/google/caja/parser/quasiliteral/RewriterMessageType.java#newcode191 src/com/google/caja/parser/quasiliteral/RewriterMessageType.java:191: TOP_LEVEL_FUNC_INCOMPATIBLE_WITH_CAJA( On 2012/12/26 23:31:13, Jasvir wrote: > In a ...
1 year, 3 months ago #25
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
1 year, 3 months ago #26
Jasvir
https://codereview.appspot.com/6827077/diff/29022/src/com/google/caja/parser/quasiliteral/RewriterMessageType.java File src/com/google/caja/parser/quasiliteral/RewriterMessageType.java (right): https://codereview.appspot.com/6827077/diff/29022/src/com/google/caja/parser/quasiliteral/RewriterMessageType.java#newcode191 src/com/google/caja/parser/quasiliteral/RewriterMessageType.java:191: TOP_LEVEL_FUNC_INCOMPATIBLE_WITH_CAJA( On 2013/01/02 18:59:16, kpreid2 wrote: > On 2012/12/26 ...
1 year, 3 months ago #27
kpreid2
https://codereview.appspot.com/6827077/diff/54001/src/com/google/caja/ses/exportsToSES.js File src/com/google/caja/ses/exportsToSES.js (right): https://codereview.appspot.com/6827077/diff/54001/src/com/google/caja/ses/exportsToSES.js#newcode28 src/com/google/caja/ses/exportsToSES.js:28: ses.rewriter___ = {}; On 2013/01/02 19:42:59, Jasvir wrote: > ...
1 year, 3 months ago #28
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
1 year, 3 months ago #29
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
1 year, 3 months ago #30
Jasvir
snapshot https://codereview.appspot.com/6827077/diff/54001/src/com/google/caja/ses/exportsToSES.js File src/com/google/caja/ses/exportsToSES.js (right): https://codereview.appspot.com/6827077/diff/54001/src/com/google/caja/ses/exportsToSES.js#newcode28 src/com/google/caja/ses/exportsToSES.js:28: ses.rewriter___ = {}; On 2013/01/02 19:48:32, kpreid2 wrote: ...
1 year, 3 months ago #31
kpreid2
LGTM, whew, yay.
1 year, 3 months ago #32
MarkM
1 year, 3 months ago #33
https://codereview.appspot.com/6827077/diff/54001/src/com/google/caja/ses/exp...
File src/com/google/caja/ses/exportsToSES.js (right):

https://codereview.appspot.com/6827077/diff/54001/src/com/google/caja/ses/exp...
src/com/google/caja/ses/exportsToSES.js:27: (function (ses, global) {
On 2013/01/02 19:42:59, Jasvir wrote:
> I can't use strict and delete global.var in Chrome.

Why not?

https://codereview.appspot.com/6827077/diff/56003/src/com/google/caja/ses/exp...
File src/com/google/caja/ses/exportsToSES.js (right):

https://codereview.appspot.com/6827077/diff/56003/src/com/google/caja/ses/exp...
src/com/google/caja/ses/exportsToSES.js:18: * environment clean.  The methods
are added to ses.rewriter since
It looks like you changed the comment to "ses.rewriter" but changed the code to
"ses.rewriter_" (single underbar).
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1274:9b9295c7fd64