http://codereview.appspot.com/157093/diff/1/5 File src/com/google/caja/cajita.js (right):
http://codereview.appspot.com/157093/diff/1/5#newcode98 src/com/google/caja/cajita.js:98: if (end === void 0) { end = self.length; } On 2009/11/19 23:54:33, MikeSamuel wrote:
Ok, so bug 1162 says that not supplying start doesn't cause problemsm
but
supplying undefined for end screws up firefox iff start is supplied. Is there a reason not to infer 0 for start as well?
I was thinking that it's unnecessary code if the browser's Array.prototype.slice does the right thing, but I just realized I'm not checking the weird failure cases on IE, so I'll fix it. http://codereview.appspot.com/157093/diff/1/2 File tests/com/google/caja/plugin/domita_test.html (right): http://codereview.appspot.com/157093/diff/1/2#newcode69 tests/com/google/caja/plugin/domita_test.html:69: s.text = 'globalSideEffect()'; On 2009/11/19 23:54:33, MikeSamuel wrote:
why text? not nodeValue or innerHTML?
setting s.innerHTML causes a runtime error on IE. s.text works on all browsers I tried. I didn't try nodeValue. http://codereview.appspot.com/157093/diff/1/3 File tests/com/google/caja/plugin/domita_test_untrusted.html (right): http://codereview.appspot.com/157093/diff/1/3#newcode119 tests/com/google/caja/plugin/domita_test_untrusted.html:119: <p>After <code>document.write</code></p> On 2009/11/19 23:54:33, MikeSamuel wrote:
This shouldn't cause a bug. If it does, it's a bug in the Parser.
it affected the .innerHTML value in IE in a weird way, something about spacing. I wasn't sure how to normalize it in a way that wouldn't break something else. it seemed simpler to close the </p> to remove the whitespace issue. http://codereview.appspot.com/157093/diff/1/3#newcode1014 tests/com/google/caja/plugin/domita_test_untrusted.html:1014: var html = el.innerHTML.replace(new RegExp('[\\r\\n]\\s*', 'g'), ''); On 2009/11/19 23:54:33, MikeSamuel wrote:
I'm fine if this is aggressive canonicalization since we're not
testing
whitespace here, but aren't newlines and spaces usually collapsed to a
single
space?
the problem is browsers seem inconsistent about whether they keep spaces or not. without that regexp replace, the innerHTML in IE has all sorts of extra spacing that doesn't show in the innerHTML in other browsers. http://codereview.appspot.com/157093/diff/1/3#newcode1828 tests/com/google/caja/plugin/domita_test_untrusted.html:1828: //assertArray([9], Array.slice([9], null, void 0), '([9], null, void 0)'); On 2009/11/19 23:54:33, MikeSamuel wrote:
I'm confused. Do we correct the problem and follow ES5? Is there a bug filed against firefox?
yeah, I'm confused too, the problem case is IE. will fix and clarify. haven't filed a firefox bug yet. http://codereview.appspot.com/157093
