Complete taming of Google APIs

[ihab.awad, 2012-08-14 14:33:06]

Jasvir and Kevin -- please look at this when you have a chance. Thanks!

[kpreid2, 2012-08-14 18:13:42]

http://codereview.appspot.com/6454148/diff/1/build.xml
File build.xml (right):

http://codereview.appspot.com/6454148/diff/1/build.xml#newcode784
build.xml:784: <replace dir="${testlib}/com/google/caja/plugin"
Misindentation.

http://codereview.appspot.com/6454148/diff/1/build.xml#newcode832
build.xml:832: <input file="${src}/com/google/caja/apitaming/test1.js"/>
Left in test code? If not, give this target a more descriptive name and docs.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
File src/com/google/caja/apitaming/cajaTamingGoogleLoader.js (right):

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:14: 
General comment: Shouldn't some of this generic taming logic be made available
to clients outside of this Google API taming?

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:16: * ...
At least some sketchy words and a TODO, please.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:21: function StringMap()
{
It would be nice not to reimplement this but I can see the need.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:23: try { o =
Object.create(undefined); } catch (e) { o = {}; }
ITYM Object.create(null).

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:24: function safe(k) {
return '[' + k + '___'; }
What is the goal of the ___ suffix here? If o does not leak, it gives no
benefit; if o does leak, then it can be used to cheat (consider e.g. the key
"foo_w" and the property name "[foo", resulting in confusion with the ES5/3
magic "[foo_w___").

How about using k + '$' as ses/StringMap.js does?

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:58: }
This allows the storing of a value but not retrieval thereof. Please clarify.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:71: cbs[i].apply({},
[]);
Why not .call({})?

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:73: console.log('Event
handler threw: ', e);
Remove the space after the colon; console args are displayed with spaces.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:99:
console.log('copyArray(' + o + ')');
Debug cruft. 3 more occurrences below.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:122: if (o === undefined
|| o === null) { return o; }
Unnecessary special case?

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:176: if
(arguments.length != 1) {
!==

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:318: sup =
sup[policy.__super__[i]];
Perhaps some error checking here so a bad path or non-array does not silently
yield undefined?

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/to...
File src/com/google/caja/apitaming/todo.txt (right):

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/to...
src/com/google/caja/apitaming/todo.txt:1: Design notes for loader as part of
taming
should be uncommitted or moved to a docs/todo location or our site or wiki

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/vi...
File src/com/google/caja/apitaming/visualizationPolicyFactory.js (right):

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/vi...
src/com/google/caja/apitaming/visualizationPolicyFactory.js:68: if ((typeof
spec) === 'string') { spec = JSON.parse(spec); }
Having parentheses here does not match the style of most of our code.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/vi...
src/com/google/caja/apitaming/visualizationPolicyFactory.js:74:
spec.dataSourceUrl= frame.rewriteUri(spec.dataSourceUrl);
missing space

http://codereview.appspot.com/6454148/diff/1/tests/com/google/caja/apitaming/...
File tests/com/google/caja/apitaming/container.html (right):

http://codereview.appspot.com/6454148/diff/1/tests/com/google/caja/apitaming/...
tests/com/google/caja/apitaming/container.html:19: <title>Caja host page</title>
Descriptive title, please.

http://codereview.appspot.com/6454148/diff/1/tests/com/google/caja/plugin/Gen...
File tests/com/google/caja/plugin/GeneralBrowserTest.java (right):

http://codereview.appspot.com/6454148/diff/1/tests/com/google/caja/plugin/Gen...
tests/com/google/caja/plugin/GeneralBrowserTest.java:25: /*
debug leftover

http://codereview.appspot.com/6454148/diff/1/tests/com/google/caja/plugin/bar...
File tests/com/google/caja/plugin/barPolicyFactory.js (right):

http://codereview.appspot.com/6454148/diff/1/tests/com/google/caja/plugin/bar...
tests/com/google/caja/plugin/barPolicyFactory.js:15:
cajaTamingGoogleLoader.addPolicyFactory('bar', function(frame, utils) {
I request a note explaining where this is used, or more descriptive/searchable
names.

[ihab.awad, 2012-08-15 21:09:39]

http://codereview.appspot.com/6454148/diff/1/build.xml
File build.xml (right):

http://codereview.appspot.com/6454148/diff/1/build.xml#newcode784
build.xml:784: <replace dir="${testlib}/com/google/caja/plugin"
On 2012/08/14 18:13:42, kpreid2 wrote:
> Misindentation.

Done.

http://codereview.appspot.com/6454148/diff/1/build.xml#newcode832
build.xml:832: <input file="${src}/com/google/caja/apitaming/test1.js"/>
On 2012/08/14 18:13:42, kpreid2 wrote:
> Left in test code? If not, give this target a more descriptive name and docs.

Y U NO LIKE MY TARGET NAME "temp"???????????

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
File src/com/google/caja/apitaming/cajaTamingGoogleLoader.js (right):

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:14: 
On 2012/08/14 18:13:42, kpreid2 wrote:
> General comment: Shouldn't some of this generic taming logic be made available
> to clients outside of this Google API taming?

Said worthy goal would be trivially achievable were JavaScript endowed with a
module system. Unless and until, I just put what I needed here; we can refactor
and add a build step to bundle everything later if necessary.

Either way, my goal is for us to publish *one* file, cajaTamingGoogleLoader.js,
which exports only *one* symbol, cajaTamingGoogleLoader. This can be <script
src>-ed by a client and allows them to add Googlez to their Cajases without
unnecessary pollution.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:16: * ...
On 2012/08/14 18:13:42, kpreid2 wrote:
> At least some sketchy words and a TODO, please.

Done.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:21: function StringMap()
{
On 2012/08/14 18:13:42, kpreid2 wrote:
> It would be nice not to reimplement this but I can see the need.

Done.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:23: try { o =
Object.create(undefined); } catch (e) { o = {}; }
On 2012/08/14 18:13:42, kpreid2 wrote:
> ITYM Object.create(null).

Done.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:24: function safe(k) {
return '[' + k + '___'; }
On 2012/08/14 18:13:42, kpreid2 wrote:
> What is the goal of the ___ suffix here? If o does not leak, it gives no
> benefit; if o does leak, then it can be used to cheat (consider e.g. the key
> "foo_w" and the property name "[foo", resulting in confusion with the ES5/3
> magic "[foo_w___").
> 
> How about using k + '$' as ses/StringMap.js does?

Done.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:58: }
On 2012/08/14 18:13:42, kpreid2 wrote:
> This allows the storing of a value but not retrieval thereof. Please clarify.

Was from a different chunk of code. Fixed.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:71: cbs[i].apply({},
[]);
On 2012/08/14 18:13:42, kpreid2 wrote:
> Why not .call({})?

I was just using .apply(...) everywhere for consistency. Changed to .call(...)
everywhere since all my calls were fixed arity.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:73: console.log('Event
handler threw: ', e);
On 2012/08/14 18:13:42, kpreid2 wrote:
> Remove the space after the colon; console args are displayed with spaces.

Done.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:99:
console.log('copyArray(' + o + ')');
On 2012/08/14 18:13:42, kpreid2 wrote:
> Debug cruft. 3 more occurrences below.

Done.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:122: if (o === undefined
|| o === null) { return o; }
On 2012/08/14 18:13:42, kpreid2 wrote:
> Unnecessary special case?

Done.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:176: if
(arguments.length != 1) {
On 2012/08/14 18:13:42, kpreid2 wrote:
> !==

Done.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:318: sup =
sup[policy.__super__[i]];
On 2012/08/14 18:13:42, kpreid2 wrote:
> Perhaps some error checking here so a bad path or non-array does not silently
> yield undefined?

Done.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/to...
File src/com/google/caja/apitaming/todo.txt (right):

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/to...
src/com/google/caja/apitaming/todo.txt:1: Design notes for loader as part of
taming
On 2012/08/14 18:13:42, kpreid2 wrote:
> should be uncommitted or moved to a docs/todo location or our site or wiki

Uncommitted. This was just random notes to myself. :)

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/vi...
File src/com/google/caja/apitaming/visualizationPolicyFactory.js (right):

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/vi...
src/com/google/caja/apitaming/visualizationPolicyFactory.js:68: if ((typeof
spec) === 'string') { spec = JSON.parse(spec); }
On 2012/08/14 18:13:42, kpreid2 wrote:
> Having parentheses here does not match the style of most of our code.

Done.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/vi...
src/com/google/caja/apitaming/visualizationPolicyFactory.js:74:
spec.dataSourceUrl= frame.rewriteUri(spec.dataSourceUrl);
On 2012/08/14 18:13:42, kpreid2 wrote:
> missing space

Done.

http://codereview.appspot.com/6454148/diff/1/tests/com/google/caja/apitaming/...
File tests/com/google/caja/apitaming/container.html (right):

http://codereview.appspot.com/6454148/diff/1/tests/com/google/caja/apitaming/...
tests/com/google/caja/apitaming/container.html:19: <title>Caja host page</title>
On 2012/08/14 18:13:42, kpreid2 wrote:
> Descriptive title, please.

Done.

http://codereview.appspot.com/6454148/diff/1/tests/com/google/caja/plugin/Gen...
File tests/com/google/caja/plugin/GeneralBrowserTest.java (right):

http://codereview.appspot.com/6454148/diff/1/tests/com/google/caja/plugin/Gen...
tests/com/google/caja/plugin/GeneralBrowserTest.java:25: /*
On 2012/08/14 18:13:42, kpreid2 wrote:
> debug leftover

Done.

http://codereview.appspot.com/6454148/diff/1/tests/com/google/caja/plugin/bar...
File tests/com/google/caja/plugin/barPolicyFactory.js (right):

http://codereview.appspot.com/6454148/diff/1/tests/com/google/caja/plugin/bar...
tests/com/google/caja/plugin/barPolicyFactory.js:15:
cajaTamingGoogleLoader.addPolicyFactory('bar', function(frame, utils) {
On 2012/08/14 18:13:42, kpreid2 wrote:
> I request a note explaining where this is used, or more descriptive/searchable
> names.

Done.

[kpreid2, 2012-08-15 22:06:16]

Looks good except for comments below and that I don't feel qualified to review
the soundness of the taming logic. Deferring to Jasvir.

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
File src/com/google/caja/apitaming/cajaTamingGoogleLoader.js (right):

http://codereview.appspot.com/6454148/diff/1/src/com/google/caja/apitaming/ca...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:73: console.log('Event
handler threw: ', e);
On 2012/08/15 21:09:39, ihab.awad wrote:
> On 2012/08/14 18:13:42, kpreid2 wrote:
> > Remove the space after the colon; console args are displayed with spaces.
> 
> Done.

Does not appear to have been done.

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
File src/com/google/caja/apitaming/visualizationPolicyFactory.js (right):

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
src/com/google/caja/apitaming/visualizationPolicyFactory.js:18: * \@overrides
window
Cruft?

[Jasvir, 2012-08-16 17:06:17]

LGTM

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
File src/com/google/caja/apitaming/cajaTamingGoogleLoader.js (right):

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:19: var
cajaTamingGoogleLoader = (function() {
Can we put this on the caja object please?

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:46: cb(unsafe(k));
Note: calling unsafe which doesn't check that k.slice(0,1) is actually ok since
the only keys that are passed to unsafe are definitely prefixed with $.  Things
inherited from Object if Object.create(null) fails are non-emumerable.

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:54: var m =
caja.iframe.contentWindow.WeakMap();
What guarantees the existence of WeakMap?

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:75: cbs[i].call({});
Note to self: Dummy object since we're not in strict mode here.

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:159: return
/__$/.test(key) ? void 0 : value;
Any reason to mix the use of void 0 and undefined in this method?

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:430: var onloads =
EventSource();
Ah.  This was confusing till I realized there's an EventSource in the local
scope and that this is is not http://dev.w3.org/html5/eventsource/.  Consider
renaming.

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:444: // trying to break
us by supplying a dumb API name like '__proto__'.
nice

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/plugin/gu...
File src/com/google/caja/plugin/guest-manager.js (right):

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/plugin/gu...
src/com/google/caja/plugin/guest-manager.js:86: rewriteUri: domicile ?
domicile.rewriteUri : function() { return null; },
Note to self: Check where this is called

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/plugin/ta...
File src/com/google/caja/plugin/taming-membrane.js (right):

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/plugin/ta...
src/com/google/caja/plugin/taming-membrane.js:128: var fclass =
({}).toString.call(f);
Object.prototype.toString to be consistent with other calls?

http://codereview.appspot.com/6454148/diff/8002/tests/com/google/caja/apitami...
File tests/com/google/caja/apitaming/container.js (right):

http://codereview.appspot.com/6454148/diff/8002/tests/com/google/caja/apitami...
tests/com/google/caja/apitaming/container.js:79: t.signalOnload();
Ihab do run() calls and window.onload in cajoled code still fire in the wrong
order in es53 vs es5 mode?

http://codereview.appspot.com/6454148/diff/8002/tests/com/google/caja/apitami...
File tests/com/google/caja/apitaming/guest.js (right):

http://codereview.appspot.com/6454148/diff/8002/tests/com/google/caja/apitami...
tests/com/google/caja/apitaming/guest.js:121: log('<font color="red">test ' +
tests[i].name + ' threw ' + e + ' - ' + e.stack + '</font>');
wrap

[ihab.awad, 2012-08-16 23:00:11]

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
File src/com/google/caja/apitaming/cajaTamingGoogleLoader.js (right):

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:19: var
cajaTamingGoogleLoader = (function() {
On 2012/08/16 17:06:17, Jasvir wrote:
> Can we put this on the caja object please?

Done.

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:46: cb(unsafe(k));
On 2012/08/16 17:06:17, Jasvir wrote:
> Note: calling unsafe which doesn't check that k.slice(0,1) is actually ok
since
> the only keys that are passed to unsafe are definitely prefixed with $. 
Things
> inherited from Object if Object.create(null) fails are non-emumerable.

Done.

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:54: var m =
caja.iframe.contentWindow.WeakMap();
On 2012/08/16 17:06:17, Jasvir wrote:
> What guarantees the existence of WeakMap?

It's defined by our "taming-frame" JS for both ES5 and ES5/3. The code here is
invoked via applyToFrame(), which is valid iff a Caja "frame" has been created,
which is possible iff the Caja taming frame is loaded.

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:75: cbs[i].call({});
On 2012/08/16 17:06:17, Jasvir wrote:
> Note to self: Dummy object since we're not in strict mode here.

Done.

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:159: return
/__$/.test(key) ? void 0 : value;
On 2012/08/16 17:06:17, Jasvir wrote:
> Any reason to mix the use of void 0 and undefined in this method?

No, just an oversight. Fixed.

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:430: var onloads =
EventSource();
On 2012/08/16 17:06:17, Jasvir wrote:
> Ah.  This was confusing till I realized there's an EventSource in the local
> scope and that this is is not http://dev.w3.org/html5/eventsource/.  Consider
> renaming.

Done.

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
src/com/google/caja/apitaming/cajaTamingGoogleLoader.js:444: // trying to break
us by supplying a dumb API name like '__proto__'.
On 2012/08/16 17:06:17, Jasvir wrote:
> nice

Done.

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
File src/com/google/caja/apitaming/visualizationPolicyFactory.js (right):

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/apitaming...
src/com/google/caja/apitaming/visualizationPolicyFactory.js:18: * \@overrides
window
On 2012/08/15 22:06:16, kpreid2 wrote:
> Cruft?

Done. Was considering Closure compilation of this at some point.

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/plugin/gu...
File src/com/google/caja/plugin/guest-manager.js (right):

http://codereview.appspot.com/6454148/diff/8002/src/com/google/caja/plugin/gu...
src/com/google/caja/plugin/guest-manager.js:86: rewriteUri: domicile ?
domicile.rewriteUri : function() { return null; },
On 2012/08/16 17:06:17, Jasvir wrote:
> Note to self: Check where this is called

Done.

http://codereview.appspot.com/6454148/diff/8002/tests/com/google/caja/apitami...
File tests/com/google/caja/apitaming/container.js (right):

http://codereview.appspot.com/6454148/diff/8002/tests/com/google/caja/apitami...
tests/com/google/caja/apitaming/container.js:79: t.signalOnload();
On 2012/08/16 17:06:17, Jasvir wrote:
> Ihab do run() calls and window.onload in cajoled code still fire in the wrong
> order in es53 vs es5 mode?

A quick test reveals that window.onload is called first, then run() invokes its
callback, in both ES5/3 and ES5.

That said, _this_ onload() is different -- it's the google.* APIs' version.

http://codereview.appspot.com/6454148/diff/8002/tests/com/google/caja/apitami...
File tests/com/google/caja/apitaming/guest.js (right):

http://codereview.appspot.com/6454148/diff/8002/tests/com/google/caja/apitami...
tests/com/google/caja/apitaming/guest.js:121: log('<font color="red">test ' +
tests[i].name + ' threw ' + e + ' - ' + e.stack + '</font>');
On 2012/08/16 17:06:17, Jasvir wrote:
> wrap

Done.

[ihab.awad, 2012-08-20 00:44:54]

@5017