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

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:
11 years, 4 months ago by Jasvir
Modified:
11 years, 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
11 years, 4 months ago (2012-11-12 17:49:16 UTC) #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 ...
11 years, 4 months ago (2012-11-12 18:06:42 UTC) #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 ...
11 years, 4 months ago (2012-11-12 18:11:45 UTC) #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" ...
11 years, 4 months ago (2012-11-12 18:16:08 UTC) #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 ...
11 years, 4 months ago (2012-11-12 19:04:24 UTC) #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 ...
11 years, 4 months ago (2012-11-12 19:16:23 UTC) #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: > ...
11 years, 4 months ago (2012-11-19 14:39:55 UTC) #7
MarkM
The stuff under src/com/google/caja/ses/* LGTM. Due to time limitations I abstain on the rest.
11 years, 4 months ago (2012-11-19 18:44:51 UTC) #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 ...
11 years, 4 months ago (2012-11-19 19:06:00 UTC) #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 ...
11 years, 4 months ago (2012-11-20 19:10:04 UTC) #10
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
11 years, 3 months ago (2012-12-13 19:54:47 UTC) #11
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
11 years, 3 months ago (2012-12-18 06:57:21 UTC) #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: ...
11 years, 3 months ago (2012-12-18 16:08:23 UTC) #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 ...
11 years, 3 months ago (2012-12-18 20:04:21 UTC) #14
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
11 years, 3 months ago (2012-12-26 23:28:51 UTC) #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 ...
11 years, 3 months ago (2012-12-26 23:31:13 UTC) #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) ...
11 years, 3 months ago (2012-12-29 20:27:26 UTC) #17
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
11 years, 3 months ago (2012-12-29 21:40:15 UTC) #18
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
11 years, 3 months ago (2012-12-29 21:56:07 UTC) #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 ...
11 years, 3 months ago (2012-12-30 03:26:34 UTC) #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 ...
11 years, 3 months ago (2012-12-30 03:46:11 UTC) #21
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
11 years, 3 months ago (2012-12-30 18:19:49 UTC) #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 ...
11 years, 3 months ago (2012-12-30 18:22:30 UTC) #23
MarkM
caja/ses/* LGTM. As usual, I leave the rest to others.
11 years, 3 months ago (2012-12-30 20:13:58 UTC) #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 ...
11 years, 2 months ago (2013-01-02 18:59:15 UTC) #25
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
11 years, 2 months ago (2013-01-02 19:41:16 UTC) #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 ...
11 years, 2 months ago (2013-01-02 19:42:59 UTC) #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: > ...
11 years, 2 months ago (2013-01-02 19:48:31 UTC) #28
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
11 years, 2 months ago (2013-01-02 19:53:49 UTC) #29
Jasvir
Uses the acorn parser and escodegen serializer to rewrite: * top level var decls to ...
11 years, 2 months ago (2013-01-02 20:07:23 UTC) #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: ...
11 years, 2 months ago (2013-01-02 20:07:27 UTC) #31
kpreid2
LGTM, whew, yay.
11 years, 2 months ago (2013-01-02 20:10:41 UTC) #32
MarkM
11 years, 2 months ago (2013-01-02 20:51:16 UTC) #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 f62528b