Revision: 3670
Author: mikesamuel
Date: Thu Aug 27 16:51:24 2009
Log: Issue 1103: style applied to body in css or cascading from body in
cajoled source not working
http://codereview.appspot.com/109083
Fixes http://code.google.com/p/google-caja/issues/detail?id=1103
body { font: 13px/1.231 Arial, helvetica, clean, sans-serif }
p { font: 83%; }
The cajoler removes the body declaration, making the p declaration
relative to an inconsistent size across browsers.
For full html pages passed to cajole_html, pages that include link
elements for style, the resulting cajoled css omits the body rule
rather than reassigning it to .gadget___ { font... }
Perhaps there's an issue here with accepting 'inherit' as a value?
Regardless, removing the rule altogether seems the wrong solution.
This affects at least YUI reset.css, fonts.css, and grids.css.
fonts.css (used to normalize element font sizes across browsers)
specifically surfaces a use case like that noted above.
[email protected]
http://code.google.com/p/google-caja/source/detail?r=3670
Modified:
/trunk/src/com/google/caja/plugin/CssRuleRewriter.java
/trunk/src/com/google/caja/plugin/domita.js
/trunk/tests/com/google/caja/plugin/CssRuleRewriterTest.java
/trunk/tests/com/google/caja/plugin/domita_test.html
=======================================
--- /trunk/src/com/google/caja/plugin/CssRuleRewriter.java Mon Jun 1
16:50:26 2009
+++ /trunk/src/com/google/caja/plugin/CssRuleRewriter.java Thu Aug 27
16:51:24 2009
@@ -102,12 +102,24 @@
// by this rule.
CssTree.SimpleSelector baseSelector = (CssTree.SimpleSelector)
sel.children().get(0);
- // If this selector is like body.ie or body.firefox, move over
- // it so that it remains topmost.
- if (sel.children().size() > 2
- && selectorMatchesElement(baseSelector, "body")
- && isDescendant(sel.children().get(1))) {
- baseSelector = (CssTree.SimpleSelector)
sel.children().get(2);
+ boolean baseIsDescendant = true;
+ if (selectorMatchesElement(baseSelector, "body")) {
+ if (sel.children().size() > 2
+ && isDescendant(sel.children().get(1))) {
+ // If this selector is like body.ie or body.firefox, move
over
+ // it so that it remains topmost.
+ baseSelector = (CssTree.SimpleSelector)
sel.children().get(2);
+ } else {
+ // Otherwise, rewrite it to use the class name that is
typically
+ // attached to virtual document bodies.
+ CssTree.IdentLiteral elName = (CssTree.IdentLiteral)
+ baseSelector.children().get(0);
+ baseSelector.replaceChild(
+ new CssTree.ClassLiteral(
+ elName.getFilePosition(), ".vdoc-body___"),
+ elName);
+ baseIsDescendant = false;
+ }
}
// Use the start position of the base selector as the position
of
@@ -115,18 +127,22 @@
FilePosition pos = FilePosition.startOf(
baseSelector.getFilePosition());
- CssTree.Combination op = new CssTree.Combination(
- pos, CssTree.Combinator.DESCENDANT);
-
CssTree.ClassLiteral restrictClass = new CssTree.ClassLiteral(
pos, "." + gadgetNameSuffix);
- CssTree.SimpleSelector restrictSel = new
CssTree.SimpleSelector(
- pos, Collections.singletonList(restrictClass));
-
- sel.createMutation()
- .insertBefore(op, baseSelector)
- .insertBefore(restrictSel, op)
- .execute();
+
+ if (baseIsDescendant) {
+ CssTree.Combination op = new CssTree.Combination(
+ pos, CssTree.Combinator.DESCENDANT);
+ CssTree.SimpleSelector restrictSel = new
CssTree.SimpleSelector(
+ pos, Collections.singletonList(restrictClass));
+
+ sel.createMutation()
+ .insertBefore(op, baseSelector)
+ .insertBefore(restrictSel, op)
+ .execute();
+ } else {
+ baseSelector.appendChild(restrictClass);
+ }
return false;
}
}, null);
=======================================
--- /trunk/src/com/google/caja/plugin/domita.js Thu Aug 27 16:44:49 2009
+++ /trunk/src/com/google/caja/plugin/domita.js Thu Aug 27 16:51:24 2009
@@ -1047,14 +1047,11 @@
function tameRelatedNode(node, editable, tameNodeCtor) {
if (node === null || node === void 0) { return null; }
- // catch errors because node might be from a different domain
+ // Catch errors because node might be from a different domain.
try {
var docElem = node.ownerDocument.documentElement;
for (var ancestor = node; ancestor; ancestor =
ancestor.parentNode) {
- // TODO(mikesamuel): replace with cursors so that subtrees are
- // delegable.
- // TODO: handle multiple classes.
- if (idClass === ancestor.className) {
+ if (idClassPattern.test(ancestor.className)) {
return tameNodeCtor(node, editable);
} else if (ancestor === docElem) {
return null;
@@ -3575,6 +3572,8 @@
throw new Error('id suffix "' + idSuffix + '" must start with "-"');
}
var idClass = idSuffix.substring(1);
+ var idClassPattern = new RegExp(
+ '(?:^|\\s)' + idClass.replace(/[\.$]/g, '\\$&') + '(?:\\s|$)');
/** A per-gadget class used to separate style rules. */
imports.getIdClass___ = function () {
return idClass;
=======================================
--- /trunk/tests/com/google/caja/plugin/CssRuleRewriterTest.java Thu Jul 23
09:16:30 2009
+++ /trunk/tests/com/google/caja/plugin/CssRuleRewriterTest.java Thu Aug 27
16:51:24 2009
@@ -52,9 +52,12 @@
assertCompiledCss(
"body.ie6#zoicks p {color:blue}",
"[ 'body.ie6#zoicks-', ' .', ' p {\\n color: blue\\n}' ]");
- assertCompiledCss( // Body markers cannot apply to the body directly.
+ assertCompiledCss( // Body markers do not apply to the body directly.
"body.ie6 {color:blue}",
- "[ '.', ' body.ie6 {\\n color: blue\\n}' ]");
+ "[ '.vdoc-body___.ie6.', ' {\\n color: blue\\n}' ]");
+ assertCompiledCss(
+ "body { font-size: 12pt }",
+ "[ '.vdoc-body___.', ' {\\n font-size: 12pt\\n}' ]");
}
public final void testCompoundRule() {
=======================================
--- /trunk/tests/com/google/caja/plugin/domita_test.html Wed Aug 26
10:01:17 2009
+++ /trunk/tests/com/google/caja/plugin/domita_test.html Thu Aug 27
16:51:24 2009
@@ -96,7 +96,7 @@
<!-- Ensure http://www.google.com/favicon.ico is :visited -->
<img src="http://www.google.com/favicon.ico"/>
- <div id="untrusted_content" class="xyz___"
+ <div id="untrusted_content" class="vdoc-body___ xyz___"
title="<Untrusted Content Title>"
style="position: relative;
width: 600px;
@@ -459,7 +459,7 @@
window.toString = function () { return '[WINDOW]'; };
inlineHtml(isValija ? 'domita_test.vo.html' : 'domita_test.co.html',
- testDomContainer);
+ testDomContainer);
})(); </script>
</body>
