Start trying to make linebreak handling sane
authorAryeh Gregor <AryehGregor+gitcommit@gmail.com>
Sun, 19 Jun 2011 15:39:27 -0600
changeset 298 7ce14572f0b5
parent 297 b8bed880fd3b
child 299 ffcb5ceacab0
Start trying to make linebreak handling sane

This makes insertLineBreak work for "<span>foo[]</span>". I'm going to
try to systematically replace as many of the totally ad-hoc br-related
things as I can with solid definitions rooted in CSS.
editcommands.html
implementation.js
source.html
tests.js
--- a/editcommands.html	Sun Jun 19 14:39:00 2011 -0600
+++ b/editcommands.html	Sun Jun 19 15:39:27 2011 -0600
@@ -435,56 +435,15 @@
 that editing commands will only modify the editing host's contents and not the
 editing host itself.
 
-<!-- I don't remember why I wrote this.  Keeping it around just in case it
-turns out to be useful.
-
-<p><var title>node</var> is an <dfn>extraneous line break</dfn> if the following
-algorithm returns true:
-
-<p class=XXX>This is positively horrible.  It attempts to move complicated CSS
-logic into DOM methods, and does so badly.  Can we somehow make this less evil,
-preferably much less evil?
-
-<ol>
-  <li>If <var title>node</var> is not a <code data-anolis-spec=html title="the br element">br</code>, return false.
-
-  <li>Let <var title>ancestor block</var> be <var title>node</var>.
-
-  <li>While <var title>ancestor block</var> is an <span>inline node</span>, set
-  <var title>ancestor block</var> to its <span data-anolis-spec=domcore title=concept-tree-parent>parent</span>.
-
-  <li>Let <var title>previous box</var> be <var title>node</var>.
-
-  <li>While <var title>previous box</var> is equal to or an <span data-anolis-spec=domcore title=concept-tree-ancestor>ancestor</span> of
-  <var title>node</var>, set <var title>previous box</var> to the <span data-anolis-spec=domcore title=concept-node>node</span> immediately
-  before it in <span data-anolis-spec=domcore>tree order</span>, or null if there is no such <span data-anolis-spec=domcore title=concept-node>node</span>.
-
-  <li>Let <var title>next box</var> be the <span data-anolis-spec=domcore title=concept-node>node</span> immediately after <var title>node</var>
-  in <span data-anolis-spec=domcore>tree order</span>, or null if there is such <span data-anolis-spec=domcore title=concept-node>node</span>.
-
-  <li>If <var title>previous box</var> is null, or is not a <span data-anolis-spec=domcore title=concept-tree-descendant>descendant</span> of
-  <var title>ancestor block</var>, or is not an <span>inline node</span>, or is a
-  <code data-anolis-spec=html title="the br element">br</code>, return false.
-  <!- -
-  This means br either is the first thing in its block container that generates
-  an inline box, or it's the first thing after another block container, or the
-  first thing after a br.  In any case, the line break will be visible.
-
-  Otherwise, it will be invisible as long as it immediately precedes a block
-  box boundary.
-  - ->
-
-  <li>If <var title>ancestor block</var> is not null and <var title>node</var> is the last
-  <span data-anolis-spec=domcore title=concept-tree-descendant>descendant</span> of <var title>ancestor block</var>, return true.
-  <!- - Precedes the end of ancestor block's box. - ->
-
-  <li>If <var title>next box</var> is not null and not an <span>inline node</span>,
-  return true.
-  <!- - Precedes the start of next box's box. - ->
-
-  <li>Return false.
-</ol>
--->
+<p>A <dfn id=collapsed-line-break>collapsed line break</dfn> is a <code class=external data-anolis-spec=html title="the br element"><a href=http://www.whatwg.org/html/#the-br-element>br</a></code> that begins a line box which
+has nothing else in it, and therefore has zero height.
+
+<p class=XXX>Is this a good definition at all?  I mean things like
+&lt;p&gt;foo&lt;br&gt;&lt;/p&gt;, or the second one in &lt;p&gt;foo&lt;br&gt;&lt;br&gt;&lt;/p&gt;.
+The way I test it is by adding a text node after it containing a zwsp; if that
+changes the offsetHeight, I deem it collapsed.  But what if it happens to be
+display: none right now, for instance?  Would it be better to use some
+DOM-based definition?
 
 <p>Each <code class=external data-anolis-spec=html><a href=http://www.whatwg.org/html/#htmldocument>HTMLDocument</a></code> has a boolean <dfn id=css-styling-flag>CSS styling flag</dfn> associated
 with it, which must initially be false.  (<a href=#the-stylewithcss-command>The <code title="">styleWithCSS</code> command</a> can be used to modify or query it, by
@@ -5245,11 +5204,10 @@
   <var title="">br</var>'s <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-parent title=concept-tree-parent>parent</a> as the first argument and one plus <var title="">br</var>'s
   <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-indexof title=concept-indexof>index</a> as the second argument.
 
-  <li>If <var title="">br</var>'s <code class=external data-anolis-spec=domcore title=dom-Node-nextSibling><a href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#dom-node-nextsibling>nextSibling</a></code> is null and <var title="">br</var>'s <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-parent title=concept-tree-parent>parent</a>
-  is not an <a href=#inline-node>inline node</a>, or if <var title="">br</var>'s <code class=external data-anolis-spec=domcore title=dom-Node-nextSibling><a href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#dom-node-nextsibling>nextSibling</a></code> is
-  not null and not an <a href=#inline-node>inline node</a>, 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("br")</a></code> on
-  the <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#context-object>context object</a> and let <var title="">extra br</var> be the result, then call
-  <code class=external data-anolis-spec=domrange title=dom-Range-insertNode><a href=http://html5.org/specs/dom-range.html#dom-range-insertnode>insertNode(<var title="">extra br</var>)</a></code> on the <a href=#active-range>active range</a>.
+  <li>If <var title="">br</var> is a <a href=#collapsed-line-break>collapsed line break</a>, 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("br")</a></code> on the <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#context-object>context object</a> and let <var title="">extra br</var>
+  be the result, then call <code class=external data-anolis-spec=domrange title=dom-Range-insertNode><a href=http://html5.org/specs/dom-range.html#dom-range-insertnode>insertNode(<var title="">extra br</var>)</a></code> on the
+  <a href=#active-range>active range</a>.
 </ol>
 
 
--- a/implementation.js	Sun Jun 19 14:39:00 2011 -0600
+++ b/implementation.js	Sun Jun 19 15:39:27 2011 -0600
@@ -660,6 +660,23 @@
 		&& getEditingHostOf(node1) == getEditingHostOf(node2);
 }
 
+// "A collapsed line break is a br that begins a line box which has nothing
+// else in it, and therefore has zero height."
+function isCollapsedLineBreak(br) {
+	if (!isHtmlElement(br, "br")) {
+		return false;
+	}
+
+	// Add a zwsp after it and see if that changes the height.
+	var space = document.createTextNode("\u200b");
+	var origHeight = br.parentNode.offsetHeight;
+	br.parentNode.insertBefore(space, br.nextSibling);
+	var finalHeight = br.parentNode.offsetHeight;
+	space.parentNode.removeChild(space);
+
+	return origHeight != finalHeight;
+}
+
 //@}
 
 /////////////////////////////
@@ -5375,12 +5392,10 @@
 		getActiveRange().setStart(br.parentNode, 1 + getNodeIndex(br));
 		getActiveRange().setEnd(br.parentNode, 1 + getNodeIndex(br));
 
-		// "If br's nextSibling is null and br's parent is not an inline node,
-		// or if br's nextSibling is not null and not an inline node, call
-		// createElement("br") on the context object and let extra br be the
-		// result, then call insertNode(extra br) on the active range."
-		if ((!br.nextSibling && !isInlineNode(br.parentNode))
-		|| (br.nextSibling && !isInlineNode(br.nextSibling))) {
+		// "If br is a collapsed line break, call createElement("br") on the
+		// context object and let extra br be the result, then call
+		// insertNode(extra br) on the active range."
+		if (isCollapsedLineBreak(br)) {
 			getActiveRange().insertNode(document.createElement("br"));
 
 			// Compensate for nonstandard implementations of insertNode
--- a/source.html	Sun Jun 19 14:39:00 2011 -0600
+++ b/source.html	Sun Jun 19 15:39:27 2011 -0600
@@ -377,56 +377,15 @@
 that editing commands will only modify the editing host's contents and not the
 editing host itself.
 
-<!-- I don't remember why I wrote this.  Keeping it around just in case it
-turns out to be useful.
-
-<p><var>node</var> is an <dfn>extraneous line break</dfn> if the following
-algorithm returns true:
-
-<p class=XXX>This is positively horrible.  It attempts to move complicated CSS
-logic into DOM methods, and does so badly.  Can we somehow make this less evil,
-preferably much less evil?
-
-<ol>
-  <li>If <var>node</var> is not a [[br]], return false.
-
-  <li>Let <var>ancestor block</var> be <var>node</var>.
-
-  <li>While <var>ancestor block</var> is an <span>inline node</span>, set
-  <var>ancestor block</var> to its [[parent]].
-
-  <li>Let <var>previous box</var> be <var>node</var>.
-
-  <li>While <var>previous box</var> is equal to or an [[ancestor]] of
-  <var>node</var>, set <var>previous box</var> to the [[node]] immediately
-  before it in [[treeorder]], or null if there is no such [[node]].
-
-  <li>Let <var>next box</var> be the [[node]] immediately after <var>node</var>
-  in [[treeorder]], or null if there is such [[node]].
-
-  <li>If <var>previous box</var> is null, or is not a [[descendant]] of
-  <var>ancestor block</var>, or is not an <span>inline node</span>, or is a
-  [[br]], return false.
-  <!- -
-  This means br either is the first thing in its block container that generates
-  an inline box, or it's the first thing after another block container, or the
-  first thing after a br.  In any case, the line break will be visible.
-
-  Otherwise, it will be invisible as long as it immediately precedes a block
-  box boundary.
-  - ->
-
-  <li>If <var>ancestor block</var> is not null and <var>node</var> is the last
-  [[descendant]] of <var>ancestor block</var>, return true.
-  <!- - Precedes the end of ancestor block's box. - ->
-
-  <li>If <var>next box</var> is not null and not an <span>inline node</span>,
-  return true.
-  <!- - Precedes the start of next box's box. - ->
-
-  <li>Return false.
-</ol>
--->
+<p>A <dfn>collapsed line break</dfn> is a [[br]] that begins a line box which
+has nothing else in it, and therefore has zero height.
+
+<p class=XXX>Is this a good definition at all?  I mean things like
+&lt;p>foo&lt;br>&lt;/p>, or the second one in &lt;p>foo&lt;br>&lt;br>&lt;/p>.
+The way I test it is by adding a text node after it containing a zwsp; if that
+changes the offsetHeight, I deem it collapsed.  But what if it happens to be
+display: none right now, for instance?  Would it be better to use some
+DOM-based definition?
 
 <p>Each [[htmldocument]] has a boolean <dfn>CSS styling flag</dfn> associated
 with it, which must initially be false.  (<span>The <code
@@ -5257,11 +5216,10 @@
   <var>br</var>'s [[parent]] as the first argument and one plus <var>br</var>'s
   [[index]] as the second argument.
 
-  <li>If <var>br</var>'s [[nextsibling]] is null and <var>br</var>'s [[parent]]
-  is not an <span>inline node</span>, or if <var>br</var>'s [[nextsibling]] is
-  not null and not an <span>inline node</span>, call [[createelement|"br"]] on
-  the [[contextobject]] and let <var>extra br</var> be the result, then call
-  [[insertnode|<var>extra br</var>]] on the <span>active range</span>.
+  <li>If <var>br</var> is a <span>collapsed line break</span>, call
+  [[createelement|"br"]] on the [[contextobject]] and let <var>extra br</var>
+  be the result, then call [[insertnode|<var>extra br</var>]] on the
+  <span>active range</span>.
 </ol>
 <!-- @} -->
 
--- a/tests.js	Sun Jun 19 14:39:00 2011 -0600
+++ b/tests.js	Sun Jun 19 15:39:27 2011 -0600
@@ -26,7 +26,7 @@
 	var toolbarDiv = document.createElement("div");
 	toolbarDiv.id = "toolbar";
 	// Note: this is completely not a hack at all.
-	toolbarDiv.innerHTML = "<style id=alerts>/* body > div > table > tbody > tr:not(.alert):not(:first-child) { display: none } */</style>"
+	toolbarDiv.innerHTML = "<style id=alerts>/* body > div > table > tbody > tr:not(.alert):not(:first-child):not(.active) { display: none } */</style>"
 		+ "<label><input id=alert-checkbox type=checkbox accesskey=a checked onclick='updateAlertRowStyle()'> Display rows without spec <u>a</u>lerts</label>"
 		+ "<label><input id=browser-checkbox type=checkbox accesskey=b checked onclick='localStorage[\"display-browser-tests\"] = event.target.checked'> Run <u>b</u>rowser tests as well as spec tests</label>";
 
@@ -1689,6 +1689,7 @@
 
 		'[]foo',
 		'foo[]',
+		'<span>foo[]</span>',
 		'foo[]<br>',
 		'foo[]bar',
 		'<address>[]foo</address>',
@@ -2907,6 +2908,7 @@
 
 	var tr = document.createElement("tr");
 	table.firstChild.appendChild(tr);
+	tr.className = (tr.className + " active").trim();
 
 	return tr;
 }
@@ -3073,6 +3075,8 @@
 
 function doSameCell(tr) {
 //@{
+	tr.className = (" " + tr.className + " ").replace(" active ", "").trim();
+
 	var sameCell = document.createElement("td");
 	var exception = false;
 	try {