Simplify indent
authorAryeh Gregor <AryehGregor+gitcommit@gmail.com>
Wed, 18 May 2011 13:30:19 -0600
changeset 143 e5f970cd0727
parent 142 8773fac2c993
child 144 579b0f28a2c6
Simplify indent

No longer respect the CSS styling flag, and don't try adding inline
style attributes. See comments for rationale.
autoimplementation.html
editcommands.html
implementation.js
source.html
--- a/autoimplementation.html	Wed May 18 13:03:04 2011 -0600
+++ b/autoimplementation.html	Wed May 18 13:30:19 2011 -0600
@@ -528,7 +528,8 @@
 		'<div>foo<p>[bar]</p>baz</div><p>extra',
 
 		// These mimic existing indentation in various browsers, to see how
-		// they cope with indenting twice.  This is Gecko non-CSS and Opera:
+		// they cope with indenting twice.  This is spec, Gecko non-CSS, and
+		// Opera:
 		'<blockquote><p>foo[bar]</p><p>baz</p></blockquote><p>extra',
 		'<blockquote><p>foo[bar</p><p>b]az</p></blockquote><p>extra',
 		'<blockquote><p>foo[bar]</p></blockquote><p>baz</p><p>extra',
@@ -560,20 +561,6 @@
 		'<p>[foo]<blockquote class="webkit-indent-blockquote" style="margin: 0 0 0 40px; border: none; padding: 0px"><p>bar</blockquote><p>extra',
 		'<p>[foo<blockquote class="webkit-indent-blockquote" style="margin: 0 0 0 40px; border: none; padding: 0px"><p>b]ar</blockquote><p>extra',
 
-		// Spec:
-		'<blockquote style="margin: 0 40px"><p>foo[bar]</p><p>baz</p></blockquote><p>extra',
-		'<blockquote style="margin: 0 40px"><p>foo[bar</p><p>b]az</p></blockquote><p>extra',
-		'<blockquote style="margin: 0 40px"><p>foo[bar]</p></blockquote><p>baz</p><p>extra',
-		'<blockquote style="margin: 0 40px"><p>foo[bar</p></blockquote><p>b]az</p><p>extra',
-		'<p>[foo]<blockquote style="margin: 0 40px"><p>bar</blockquote><p>extra',
-		'<p>[foo<blockquote style="margin: 0 40px"><p>b]ar</blockquote><p>extra',
-		'<div style="margin: 0 40px"><p>foo[bar]</p><p>baz</p></div><p>extra',
-		'<div style="margin: 0 40px"><p>foo[bar</p><p>b]az</p></div><p>extra',
-		'<div style="margin: 0 40px"><p>foo[bar]</p></div><p>baz</p><p>extra',
-		'<p>[foo]<div style="margin: 0 40px"><p>bar</div><p>extra',
-		'<p>[foo<div style="margin: 0 40px"><p>b]ar</div><p>extra',
-		'<div style="margin: 0 40px"><p>foo[bar</p></div><p>b]az</p><p>extra',
-
 		// MDC says "In Firefox, if the selection spans multiple lines at
 		// different levels of indentation, only the least indented lines in
 		// the selection will be indented."  Let's test that.
@@ -877,8 +864,8 @@
 	],
 	outdent: [
 		// These mimic existing indentation in various browsers, to see how
-		// they cope with outdenting various things.  This is Gecko non-CSS and
-		// Opera:
+		// they cope with outdenting various things.  This is spec, Gecko
+		// non-CSS, and Opera:
 		'<blockquote><p>foo[bar]</p><p>baz</p></blockquote><p>extra',
 		'<blockquote><p>foo[bar</p><p>b]az</p></blockquote><p>extra',
 		'<blockquote><p>foo[bar]</p></blockquote><p>baz</p><p>extra',
@@ -902,16 +889,6 @@
 		'<blockquote class="webkit-indent-blockquote" style="margin: 0 0 0 40px; border: none; padding: 0px;"><p>foo[bar]</p></blockquote><p>baz</p><p>extra',
 		'<blockquote class="webkit-indent-blockquote" style="margin: 0 0 0 40px; border: none; padding: 0px;"><p>foo[bar</p></blockquote><p>b]az</p><p>extra',
 
-		// Spec:
-		'<blockquote style="margin: 0 40px"><p>foo[bar]</p><p>baz</p></blockquote><p>extra',
-		'<blockquote style="margin: 0 40px"><p>foo[bar</p><p>b]az</p></blockquote><p>extra',
-		'<blockquote style="margin: 0 40px"><p>foo[bar]</p></blockquote><p>baz</p><p>extra',
-		'<blockquote style="margin: 0 40px"><p>foo[bar</p></blockquote><p>b]az</p><p>extra',
-		'<div style="margin: 0 40px"><p>foo[bar]</p><p>baz</p></div><p>extra',
-		'<div style="margin: 0 40px"><p>foo[bar</p><p>b]az</p></div><p>extra',
-		'<div style="margin: 0 40px"><p>foo[bar]</p></div><p>baz</p><p>extra',
-		'<div style="margin: 0 40px"><p>foo[bar</p></div><p>b]az</p><p>extra',
-
 		// Now let's try nesting lots of stuff and see what happens.
 		'<blockquote><blockquote>foo[bar]baz</blockquote></blockquote>',
 		'<blockquote><blockquote data-abc=def>foo[bar]baz</blockquote></blockquote>',
@@ -1387,7 +1364,6 @@
 	"fontname",
 	"fontsize",
 	"forecolor",
-	"indent",
 	"italic",
 	"strikethrough",
 	"subscript",
@@ -1707,7 +1683,6 @@
 		// now this includes ignoring where the selection goes.
 		var normalizedSpecCell = tr.childNodes[1].lastChild.firstChild.textContent
 			.replace(/[[\]{}]/g, "")
-			.replace(/ style="margin: 0 40px"/g, "")
 			.replace(/ class="webkit-indent-blockquote" style="margin: 0 0 0 40px; border: none; padding: 0px;"/g, '')
 			.replace(/ style="margin-right: 0px;" dir="ltr"/g, '')
 			.replace(/ style="margin-left: 0px;" dir="rtl"/g, '')
@@ -1720,7 +1695,6 @@
 			.replace(/#[0-9a-fA-F]{6}/g, function(match) { return match.toUpperCase(); });
 		var normalizedBrowserCell = tr.childNodes[2].lastChild.firstChild.textContent
 			.replace(/[[\]{}]/g, "")
-			.replace(/ style="margin: 0 40px"/g, "")
 			.replace(/ class="webkit-indent-blockquote" style="margin: 0 0 0 40px; border: none; padding: 0px;"/g, '')
 			.replace(/ style="margin-right: 0px;" dir="ltr"/g, '')
 			.replace(/ style="margin-left: 0px;" dir="rtl"/g, '')
--- a/editcommands.html	Wed May 18 13:03:04 2011 -0600
+++ b/editcommands.html	Wed May 18 13:30:19 2011 -0600
@@ -2305,23 +2305,27 @@
     <li>Abort these steps.
   </ol>
 
-  <li>Let <var title="">tag</var> be "div" if the <a href=#css-styling-flag>CSS styling flag</a> is
-  true, otherwise "blockquote".
-  <!-- Firefox 4.0 is the only tested browser that respects the CSS styling
-  flag for indent at all.  For indent, as for inline markup commands like
-  bold, it will modify the inline style of existing elements if available,
-  and only create spans/divs if there are no existing elements where it could
-  add the style instead.  For indent as for other commands, I follow WebKit
-  and always create an extra element, to ensure consistency between CSS and
-  non-CSS modes. -->
-
-  <li><a href=#wrap>Wrap</a> <var title="">node list</var>.  <a href=#sibling-criteria>Sibling criteria</a>
-  must match only an <a href=#indentation-element>indentation element</a> whose "display" property
-  computes to "block" and whose "margin-left" and "margin-right" properties
-  compute to "40px", and whose "margin-top" and "margin-bottom" properties
-  compute to "0".  The <a href=#new-parent-instructions>new parent instructions</a> are to call <code class=external data-anolis-spec=domcore title=dom-Document-createElement><a href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#dom-document-createelement>createElement(<var title="">tag</var>)</a></code> on the
-  <code class=external data-anolis-spec=domcore title=dom-Node-ownerDocument><a href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#dom-node-ownerdocument>ownerDocument</a></code> of <var title="">first node</var>, then set the CSS property
-  "margin" of the returned node to "0 40px", then return the returned node.
+  <!--
+  Firefox 4.0 respects the CSS styling flag for indent, but Chrome 12 dev does
+  not.  I always produce blockquotes, even if CSS styling is on, for two
+  reasons.  One, IE9 handles inline margin attributes badly: when outdenting,
+  it propagates the margin to the parent, which doesn't actually remove it.
+  Two, in CSS mode I'd want to use <div style="margin: 1em 40px"> to match
+  non-CSS mode, but authors are very likely to want to remove the top/bottom
+  margin, which they can't do if it's not a special tag.  Authors who really
+  want divs for indentation could always convert the blockquotes to divs
+  themselves.  But if people really want it, I could respect CSS styling mode
+  here too.
+
+  The top/bottom margins might be undesirable here, but no more so than for
+  <ol>/<ul>/<p>/etc.  Here as there, authors can remove them with CSS if they
+  want.
+  -->
+
+  <li><a href=#wrap>Wrap</a> <var title="">node list</var>, with <a href=#sibling-criteria>sibling
+  criteria</a> matching any <a href=#indentation-element>indentation element</a>, and <a href=#new-parent-instructions>new
+  parent instructions</a> to return the result of calling <code class=external data-anolis-spec=domcore title=dom-Document-createElement><a href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#dom-document-createelement>createElement("blockquote")</a></code> on the
+  <code class=external data-anolis-spec=domcore title=dom-Node-ownerDocument><a href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#dom-node-ownerdocument>ownerDocument</a></code> of <var title="">first node</var>.
 
   <p class=XXX>This indents on both sides, so we don't have to worry about
   directionality.  Preferably we should indent only on the start side, but
@@ -2329,16 +2333,8 @@
   browsers start to support margin-start and so on, we can't use them because
   a) we have to work okay in legacy browsers and b) it doesn't help if a
   descendant block has different direction (so should be indented the other
-  way).
-
-  <p class=XXX>IE9 doesn't handle an explicit margin attribute very well when
-  outdenting: it propagates it to the parent when removing the element, which
-  doesn't actually remove the indentation.  So indentation per spec (or
-  Gecko in CSS mode or WebKit) will not be removed correctly by IE.  I'm
-  leaving the style in because this short-term incompatibility is preferable
-  to the long-term incorrectness of adding top/bottom margins or adding
-  margins on both sides.  I can't think of any better way to do this at the
-  moment.
+  way).  It's not clear to me that we should worry about it: most browsers
+  don't, and the ones that do get it wrong.
 </ol>
 
 <dd><strong>State</strong>:
@@ -2971,16 +2967,15 @@
 <!--
 Things that are produced for indentation that we need to consider removing:
 
-* Plain <blockquote> (produced by Opera 11.00 and non-CSS Firefox 4.0)
+* Plain <blockquote> (produced by spec, Firefox 4.0 non-CSS, Opera 11.00)
 * <blockquote style="margin-right: 0" dir="ltr"> and <blockquote
   style="margin-left: 0" dir="rtl"> (IE9)
 * <blockquote class="webkit-indent-blockquote" style="margin: 0 0 0 40px;
   border: none; padding: 0px"> (Chrome 12 dev)
 * <div style="margin-left: 40px"> and <div style="margin-right: 40px">
-  (CSS Firefox 4.0 if no other element available)
-* <blockquote style="margin: 0 40px"> and <div style="margin: 0 40px"> (spec)
+  (Firefox 4.0 CSS if no other element available)
 * Other random things with display: block whose left or right margin was
-  increased by 40px (CSS Firefox 4.0)
+  increased by 40px (Firefox 4.0 CSS)
 
 For discussion on the list-related stuff, see the comment for
 insertOrderedList.
--- a/implementation.js	Wed May 18 13:03:04 2011 -0600
+++ b/implementation.js	Wed May 18 13:30:19 2011 -0600
@@ -3092,34 +3092,12 @@
 		return;
 	}
 
-	// "Let tag be "div" if the CSS styling flag is true, otherwise
-	// "blockquote"."
-	var tag = cssStylingFlag ? "div" : "blockquote";
-
-	// "Wrap node list. Sibling criteria must match only an indentation element
-	// whose "display" property computes to "block" and whose "margin-left" and
-	// "margin-right" properties compute to "40px", and whose "margin-top" and
-	// "margin-bottom" properties compute to "0". The new parent instructions
-	// are to call createElement(tag) on the ownerDocument of first node, then
-	// set the CSS property "margin" of the returned node to "0 40px", then
-	// return the returned node."
+	// "Wrap node list, with sibling criteria matching any indentation element,
+	// and new parent instructions to return the result of calling
+	// createElement("blockquote") on the ownerDocument of first node."
 	wrap(nodeList,
-		function(node) {
-			if (!isIndentationElement(node)) {
-				return false;
-			}
-			var style = getComputedStyle(node);
-			return style.display == "block"
-				&& style.marginLeft == "40px"
-				&& style.marginRight == "40px"
-				&& style.marginTop == "0px"
-				&& style.marginBottom == "0px";
-		},
-		function() {
-			var ret = firstNode.ownerDocument.createElement(tag);
-			ret.setAttribute("style", "margin: 0 40px");
-			return ret;
-		});
+		function(node) { return isIndentationElement(node) },
+		function() { return firstNode.ownerDocument.createElement("blockquote") });
 }
 
 function outdentNode(node) {
--- a/source.html	Wed May 18 13:03:04 2011 -0600
+++ b/source.html	Wed May 18 13:30:19 2011 -0600
@@ -2332,25 +2332,29 @@
     <li>Abort these steps.
   </ol>
 
-  <li>Let <var>tag</var> be "div" if the <span>CSS styling flag</span> is
-  true, otherwise "blockquote".
-  <!-- Firefox 4.0 is the only tested browser that respects the CSS styling
-  flag for indent at all.  For indent, as for inline markup commands like
-  bold, it will modify the inline style of existing elements if available,
-  and only create spans/divs if there are no existing elements where it could
-  add the style instead.  For indent as for other commands, I follow WebKit
-  and always create an extra element, to ensure consistency between CSS and
-  non-CSS modes. -->
-
-  <li><span>Wrap</span> <var>node list</var>.  <span>Sibling criteria</span>
-  must match only an <span>indentation element</span> whose "display" property
-  computes to "block" and whose "margin-left" and "margin-right" properties
-  compute to "40px", and whose "margin-top" and "margin-bottom" properties
-  compute to "0".  The <span>new parent instructions</span> are to call <code
+  <!--
+  Firefox 4.0 respects the CSS styling flag for indent, but Chrome 12 dev does
+  not.  I always produce blockquotes, even if CSS styling is on, for two
+  reasons.  One, IE9 handles inline margin attributes badly: when outdenting,
+  it propagates the margin to the parent, which doesn't actually remove it.
+  Two, in CSS mode I'd want to use <div style="margin: 1em 40px"> to match
+  non-CSS mode, but authors are very likely to want to remove the top/bottom
+  margin, which they can't do if it's not a special tag.  Authors who really
+  want divs for indentation could always convert the blockquotes to divs
+  themselves.  But if people really want it, I could respect CSS styling mode
+  here too.
+
+  The top/bottom margins might be undesirable here, but no more so than for
+  <ol>/<ul>/<p>/etc.  Here as there, authors can remove them with CSS if they
+  want.
+  -->
+
+  <li><span>Wrap</span> <var>node list</var>, with <span>sibling
+  criteria</span> matching any <span>indentation element</span>, and <span>new
+  parent instructions</span> to return the result of calling <code
   data-anolis-spec=domcore
-  title=dom-Document-createElement>createElement(<var>tag</var>)</code> on the
-  [[ownerdocument]] of <var>first node</var>, then set the CSS property
-  "margin" of the returned node to "0 40px", then return the returned node.
+  title=dom-Document-createElement>createElement("blockquote")</code> on the
+  [[ownerdocument]] of <var>first node</var>.
 
   <p class=XXX>This indents on both sides, so we don't have to worry about
   directionality.  Preferably we should indent only on the start side, but
@@ -2358,16 +2362,8 @@
   browsers start to support margin-start and so on, we can't use them because
   a) we have to work okay in legacy browsers and b) it doesn't help if a
   descendant block has different direction (so should be indented the other
-  way).
-
-  <p class=XXX>IE9 doesn't handle an explicit margin attribute very well when
-  outdenting: it propagates it to the parent when removing the element, which
-  doesn't actually remove the indentation.  So indentation per spec (or
-  Gecko in CSS mode or WebKit) will not be removed correctly by IE.  I'm
-  leaving the style in because this short-term incompatibility is preferable
-  to the long-term incorrectness of adding top/bottom margins or adding
-  margins on both sides.  I can't think of any better way to do this at the
-  moment.
+  way).  It's not clear to me that we should worry about it: most browsers
+  don't, and the ones that do get it wrong.
 </ol>
 
 <dd><strong>State</strong>:
@@ -3016,16 +3012,15 @@
 <!--
 Things that are produced for indentation that we need to consider removing:
 
-* Plain <blockquote> (produced by Opera 11.00 and non-CSS Firefox 4.0)
+* Plain <blockquote> (produced by spec, Firefox 4.0 non-CSS, Opera 11.00)
 * <blockquote style="margin-right: 0" dir="ltr"> and <blockquote
   style="margin-left: 0" dir="rtl"> (IE9)
 * <blockquote class="webkit-indent-blockquote" style="margin: 0 0 0 40px;
   border: none; padding: 0px"> (Chrome 12 dev)
 * <div style="margin-left: 40px"> and <div style="margin-right: 40px">
-  (CSS Firefox 4.0 if no other element available)
-* <blockquote style="margin: 0 40px"> and <div style="margin: 0 40px"> (spec)
+  (Firefox 4.0 CSS if no other element available)
 * Other random things with display: block whose left or right margin was
-  increased by 40px (CSS Firefox 4.0)
+  increased by 40px (Firefox 4.0 CSS)
 
 For discussion on the list-related stuff, see the comment for
 insertOrderedList.