I'd like you to do a code review. To review this change, run gvn review --project https://google-caja.googlecode.com/svn mikesamuel/[EMAIL PROTECTED]
Alternatively, to review the latest snapshot of this change branch, run gvn --project https://google-caja.googlecode.com/svn review mikesamuel/bug-718 to review the following change: *mikesamuel/[EMAIL PROTECTED] | mikesamuel | 2008-09-29 06:18:49 -0800 (Mon, 29 Sep 2008) Description: bug 718: <style> doesn't work in safari 3.1.2 <style>p { color: red; }</style> <p>hi</p> cajoled, the style gets applied in ff, but not safari 3.1.2, and possibly other safari versions. fix is easy. apparently safari wants you to add new style nodes into the document head. so change domita getCssContainer___ from document.body to document.getElementsByTagName('head')[0] some more details on adding stylesheets dynamically here: http://yuiblog.com/blog/2007/06/07/style/ Modified getCssContainer in domita.js to return head per the blog post referenced in the bug. Added a test to domita_test.html that styles a link green and checks that it is green using getComputedStyle. Tweaked domita_test.html and supporting JS to run under Safari. Affected Paths: M //trunk/src/com/google/caja/plugin/domita.js M //trunk/tests/com/google/caja/plugin/domita_test.html M //trunk/tests/com/google/caja/plugin/domita_test.js M //trunk/tests/com/google/caja/plugin/jsunit.js This is a semiautomated message from "gvn mail". See <http://code.google.com/p/gvn/> to learn more. Index: src/com/google/caja/plugin/domita.js =================================================================== --- src/com/google/caja/plugin/domita.js (^/trunk/src/com/google/caja/plugin/[EMAIL PROTECTED]) +++ src/com/google/caja/plugin/domita.js (^/changes/mikesamuel/bug-718/trunk/src/com/google/caja/plugin/[EMAIL PROTECTED]) @@ -1096,7 +1096,8 @@ attachDocumentStub = (function () { */ imports.emitCss___ = function (cssText) { // Courtesy Stoyan Stefanov who documents the derivation of this at - // http://www.phpied.com/dynamic-script-and-style-elements-in-ie/ + // http://www.phpied.com/dynamic-script-and-style-elements-in-ie/ and + // http://yuiblog.com/blog/2007/06/07/style/ var styleSheet = document.createElement('style'); styleSheet.setAttribute('type', 'text/css'); if (styleSheet.styleSheet) { // IE @@ -1108,7 +1109,7 @@ attachDocumentStub = (function () { }; /** The node to which gadget stylesheets should be added. */ imports.getCssContainer___ = function () { - return document.body; + return document.getElementsByTagName('head')[0]; }; var idClass = idSuffix.replace(/^-/, ''); Index: tests/com/google/caja/plugin/domita_test.html =================================================================== --- tests/com/google/caja/plugin/domita_test.html (^/trunk/tests/com/google/caja/plugin/[EMAIL PROTECTED]) +++ tests/com/google/caja/plugin/domita_test.html (^/changes/mikesamuel/bug-718/trunk/tests/com/google/caja/plugin/[EMAIL PROTECTED]) @@ -89,11 +89,24 @@ // Give unfiltered DOM access so we can check the results of actions. testImports.directAccess = ___.primFreeze({ + // Allow testing of emitHtml by exposing it for testing + emitCssHook: ___.simpleFrozenFunc(function (css) { + testImports.emitCss___(css.join('xyz___')); + }), getInnerHTML: ___.simpleFunc(function (tameNode) { return tameNode.node___.innerHTML; }), getAttribute: ___.simpleFunc(function (tameNode, name) { return tameNode.node___.getAttribute(name); + }), + getComputedStyle: ___.simpleFunc(function (tameNode, styleProp) { + var node = tameNode.node___; + if (node.currentStyle) { + return node.currentStyle[styleProp]; + } else if (window.getComputedStyle) { + return document.defaultView.getComputedStyle(node, null) + .getPropertyValue(styleProp); + } }) }); @@ -170,5 +183,11 @@ <div class="testcontainer" id="test-navigator-xyz___"></div> + <div class="xyz___"> + <div class="testcontainer" id="test-emit-css-xyz___"> + <a id="not-blue-xyz___" href="#">I am not blue</a> + </div> + </div> + </body> </html> Index: tests/com/google/caja/plugin/domita_test.js =================================================================== --- tests/com/google/caja/plugin/domita_test.js (^/trunk/tests/com/google/caja/plugin/[EMAIL PROTECTED]) +++ tests/com/google/caja/plugin/domita_test.js (^/changes/mikesamuel/bug-718/trunk/tests/com/google/caja/plugin/[EMAIL PROTECTED]) @@ -42,29 +42,51 @@ function assertFailsSafe(canFail, assertionsIfPass */ function canonInnerHtml(s) { // Sort attributes. + var htmlAttribute = new RegExp('^\\s*(\\w+)(?:\\s*=\\s*("[^\\"]*"' + + '|\'[^\\\']*\'|[^\\\'\\"\\s>]+))?'); + var quot = new RegExp('"', 'g'); + var htmlStartTag = new RegExp('(<\\w+)\\s+([^\\s>][^>]*)>', 'g'); + var htmlTag = new RegExp('(<\/?)(\\w+)(\\s+[^\\s>][^>]*)?>', 'g'); + var ignorableWhitespace = new RegExp('^[ \\t]*(\\r\\n?|\\n)|\\s+$', 'g'); + var tagEntityOrText = new RegExp( + '(?:(</?\\w[^>]*>|&[a-zA-Z#]|[^<&>]+)|([<&>]))', 'g'); s = s.replace( - new RegExp('(<\\w+)\\s+([^\\s>][^>]*)>', 'g'), + htmlStartTag, function (_, tagStart, tagBody) { var attrs = []; - for (var m; (m = tagBody.match( - new RegExp('^\\s*(\\w+)(?:\\s*=\\s*("[^\\"]*"' - + '|\'[^\\\']*\'|[^\\\'\\"\\s>]+))?')));) { - var value = m[2] && !(new RegExp('^["\']')).test(m[2]) - ? '"' + m[2] + '"' - : m[2]; - attrs.push(m[1] + (value ? '=' + value : '')); + for (var m; (m = tagBody.match(htmlAttribute));) { + var name = m[1]; + var value = m[2]; + var hasValue = value != null; + if (hasValue && (new RegExp('^["\']')).test(value)) { + value = value.substring(1, value.length - 1); + } + attrs.push( + hasValue + ? name + '="' + value.replace(quot, '"') + '"' + : name); tagBody = tagBody.substring(m[0].length); } attrs.sort(); - return tagStart + ' ' +attrs.join(' ') + '>'; + return tagStart + ' ' + attrs.join(' ') + '>'; }); s = s.replace( - new RegExp('(<\/?)(\\w+)([^>]*)>', 'g'), + htmlTag, function (_, open, name, body) { - return open + name.toLowerCase() + body + '>'; + return open + name.toLowerCase() + (body || '') + '>'; }); // Remove ignorable whitespace. - return s.replace(new RegExp('^[ \\t]*(\\r\\n?|\\n)|\\s+$', 'g'), ''); + s = s.replace(ignorableWhitespace, ''); + // Normalize escaping of text nodes since Safari doesn't escape loose >. + s = s.replace( + tagEntityOrText, + function (_, good, bad) { + return good + ? good + : (bad.replace(new RegExp('&', 'g'), '&') + .replace(new RegExp('>', 'g'), '>')); + }); + return s; } jsunitRegister('testGetElementById', @@ -117,7 +139,7 @@ jsunitRegister('testCreateElement', assertEquals('howdy <there>', text.data); newNode.appendChild(text); assertEquals(3, newNode.firstChild.nodeType); - assertEquals('howdy <there>', newNode.innerHTML); + assertEquals('howdy <there>', canonInnerHtml(newNode.innerHTML)); pass('test-create-element'); }); @@ -159,10 +181,10 @@ jsunitRegister('testForms', container.innerHTML = '<form onsubmit="foo()">' + '<input type="submit" value="Submit"></form>'; - assertEquals('<form onsubmit=\'' + assertEquals('<form onsubmit="' + 'try { plugin_dispatchEvent___' - + '(this, event || window.event, 0, "foo");' - + ' } finally { return false; }\'>' + + '(this, event || window.event, 0, "foo");' + + ' } finally { return false; }">' + '<input type="submit" value="Submit"></form>', canonInnerHtml(directAccess.getInnerHTML(container))); @@ -345,3 +367,17 @@ jsunitRegister('testNavigator', window.navigator.appCodeName + '/' + window.navigator.appVersion); pass('test-navigator'); }); + +jsunitRegister('testEmitCss', + function testCss() { + directAccess.emitCssHook(['.', ' a { color: #00ff00 }']); + var computedColor = directAccess.getComputedStyle( + document.getElementById('not-blue'), 'color').toLowerCase(); + if (!(computedColor === 'green' || computedColor === '#00ff00' + || computedColor === '#0f0' || computedColor === 'rgb(0, 255, 0)' + || computedColor === 'rgb(0, 100%, 0)')) { + fail(computedColor + ' is not green'); + } else { + pass('test-emit-css'); + } +}); Index: tests/com/google/caja/plugin/jsunit.js =================================================================== --- tests/com/google/caja/plugin/jsunit.js (^/trunk/tests/com/google/caja/plugin/[EMAIL PROTECTED]) +++ tests/com/google/caja/plugin/jsunit.js (^/changes/mikesamuel/bug-718/trunk/tests/com/google/caja/plugin/[EMAIL PROTECTED]) @@ -45,8 +45,8 @@ function jsunitRun() { var nFailures = 0; for (var i = 0; i < testNames.length; ++i) { var testName = testNames[i]; - (typeof console !== 'undefined') && (console.group('running %s', testName), - console.time(testName)); + (typeof console !== 'undefined' && 'group' in console) + && (console.group('running %s', testName), console.time(testName)); try { (typeof setUp === 'function') && setUp(); jsunitTests[testName].call(this); @@ -57,8 +57,8 @@ function jsunitRun() { (typeof console !== 'undefined') && (console.error((e.message || '' + e) + '\n' + e.stack)); } finally { - (typeof console !== 'undefined') && (console.timeEnd(testName), - console.groupEnd()); + (typeof console !== 'undefined' && 'group' in console) + && (console.timeEnd(testName), console.groupEnd()); } } @@ -69,8 +69,8 @@ function jsunitRun() { document.title = (originalTitle + ' - ' + msg); throw firstFailure || new Error(msg); } - (typeof console !== 'undefined') && (console.group(document.title), - console.groupEnd()); + (typeof console !== 'undefined' && 'group' in console) + && (console.group(document.title), console.groupEnd()); } if ('undefined' === typeof console) { --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to http://groups.google.com/group/google-caja-discuss To unsubscribe, email [EMAIL PROTECTED] -~----------~----~----~----~------~----~------~--~---
