Revamp unlink behavior to match IE
authorAryeh Gregor <AryehGregor+gitcommit@gmail.com>
Fri, 13 May 2011 15:42:04 -0600
changeset 119 fe300217c1b0
parent 118 39f401162a30
child 120 577781808581
Revamp unlink behavior to match IE

This behavior seems more logical, and doesn't require us to break up
elements either.
autoimplementation.html
editcommands.html
implementation.js
source.html
--- a/autoimplementation.html	Fri May 13 14:32:48 2011 -0600
+++ b/autoimplementation.html	Fri May 13 15:42:04 2011 -0600
@@ -271,6 +271,14 @@
 		'foo<a href=otherurl>[bar</a>baz]',
 		'[foo<a href=otherurl>bar]</a>baz',
 		'[foo<a href=otherurl>bar</a>baz]',
+
+		'<a href=otherurl><b>foo[bar]baz</b></a>',
+		'<a href=otherurl><b>foo[barbaz</b></a>}',
+		'{<a href=otherurl><b>foobar]baz</b></a>',
+		'<a href=otherurl><b>[foobarbaz]</b></a>',
+
+		'<a name=abc>foo[bar]baz</a>',
+		'<a name=abc><b>foo[bar]baz</b></a>',
 	],
 	fontname: [
 		'foo[]bar',
@@ -1296,18 +1304,21 @@
 		'{<a href=http://www.google.com/>foobarbaz</a>}',
 		'<a href=http://www.google.com/>[foobarbaz]</a>',
 
+		'foo<a href=http://www.google.com/>b[]ar</a>baz',
 		'foo<a href=http://www.google.com/>[bar]</a>baz',
 		'foo[<a href=http://www.google.com/>bar</a>]baz',
 		'foo<a href=http://www.google.com/>[bar</a>baz]',
 		'[foo<a href=http://www.google.com/>bar]</a>baz',
 		'[foo<a href=http://www.google.com/>bar</a>baz]',
 
+		'<a id=foo href=http://www.google.com/>foobar[]baz</a>',
 		'<a id=foo href=http://www.google.com/>foo[bar]baz</a>',
 		'<a id=foo href=http://www.google.com/>[foobarbaz]</a>',
 		'foo<a id=foo href=http://www.google.com/>[bar]</a>baz',
 		'foo[<a id=foo href=http://www.google.com/>bar</a>]baz',
 		'[foo<a id=foo href=http://www.google.com/>bar</a>baz]',
 
+		'<a name=foo>foobar[]baz</a>',
 		'<a name=foo>foo[bar]baz</a>',
 		'<a name=foo>[foobarbaz]</a>',
 		'foo<a name=foo>[bar]</a>baz',
--- a/editcommands.html	Fri May 13 14:32:48 2011 -0600
+++ b/editcommands.html	Fri May 13 15:42:04 2011 -0600
@@ -1774,14 +1774,15 @@
 
     <a href=http://example.com><b>A<a href=http://example.org>b</a>c</b></a>
 
-  IE 9 RC and Opera 11 produce simply:
+  (This doesn't round-trip through text/html serialization.)  IE 9 RC and Opera
+  11 produce simply:
 
     <a href=http://example.org><b>Abc</b></a>
 
-  The last behavior produces valid markup (unlike Gecko), is simple (unlike
-  WebKit), and probably best matches user expectations.  If you happen to miss
-  out a character when selecting the link you want to change, do you really
-  intend to only change the link of part of it? -->
+  The last behavior probably best matches user expectations.  If you happen to
+  miss out a character when selecting the link you want to change, do you
+  really intend to only change the link of part of it?
+  -->
 
   <li><a href=#set-the-value>Set the value</a> of each <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-node title=concept-node>node</a> in <var title="">node list</var> to
   <var title="">value</var>.
@@ -3559,10 +3560,23 @@
 
 <dt><code title=""><dfn id=command-unlink title=command-unlink>unlink</dfn></code>
 
-<dd><strong>Action</strong>: <a href=#decompose>Decompose</a> the <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-range title=concept-range>range</a>, and
-<a href=#set-the-value>set the value</a> of each returned <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-node title=concept-node>node</a> to null.
-<!-- IE 9 RC unlinks the whole link you're pointing at.  We follow the other
-browsers, which operate more consistently with other commands. -->
+<dd><strong>Action</strong>:
+<!--
+IE 9 RC unlinks the whole link you're pointing at, while others only
+unlink the current text.  The latter behavior seems less expected, as with
+createLink, although I can't articulate precisely why.  Word 2007 and
+OpenOffice.org 3.2.1 (Ubuntu) seem to give an option to remove the whole link
+or none of it, which backs the spec's requirement.  See also #whatwg logs
+starting at 2011-05-13 at 16:53 EDT (UTC-0400).
+-->
+
+<ol>
+  <li>Let <var title="">hyperlinks</var> be a list of every <code class=external data-anolis-spec=html title="the a element"><a href=http://www.whatwg.org/html/#the-a-element>a</a></code> element that has an
+  <code class=external data-anolis-spec=html title=attr-hyperlink-href><a href=http://www.whatwg.org/html/#attr-hyperlink-href>href</a></code> attribute and is <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#contained>contained</a> in the <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-range title=concept-range>range</a> or is an
+  <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-ancestor title=concept-tree-ancestor>ancestor</a> of one of its <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-boundary-point title=concept-boundary-point>boundary points</a>.
+
+  <li><a href=#clear-the-value>Clear the value</a> of each member of <var title="">hyperlinks</var>.
+</ol>
 
 <dd><strong>State</strong>: Always false.
 
--- a/implementation.js	Fri May 13 14:32:48 2011 -0600
+++ b/implementation.js	Fri May 13 15:42:04 2011 -0600
@@ -3095,11 +3095,40 @@
 		break;
 
 		case "unlink":
-		// "Decompose the range, and set the value of each returned node to
-		// null."
-		var nodeList = decomposeRange(range);
-		for (var i = 0; i < nodeList.length; i++) {
-			setNodeValue(nodeList[i], command, null);
+		// "Let hyperlinks be a list of every a element that has an href
+		// attribute and is contained in the range or is an ancestor of one of
+		// its boundary points."
+		//
+		// As usual, take care to ensure it's tree order.  The correctness of
+		// the following is left as an exercise for the reader.
+		var hyperlinks = [];
+		for (
+			var node = range.startContainer;
+			node;
+			node = node.parentNode
+		) {
+			if (isHtmlElement(node, "A")
+			&& node.hasAttribute("href")) {
+				hyperlinks.unshift(node);
+			}
+		}
+		for (
+			var node = range.startContainer;
+			node != nextNodeDescendants(range.endContainer);
+			node = nextNode(node)
+		) {
+			if (isHtmlElement(node, "A")
+			&& node.hasAttribute("href")
+			&& (isContained(node, range)
+			|| isAncestor(node, range.endContainer)
+			|| node == range.endContainer)) {
+				hyperlinks.push(node);
+			}
+		}
+
+		// "Clear the value of each member of hyperlinks."
+		for (var i = 0; i < hyperlinks.length; i++) {
+			clearValue(hyperlinks[i], command);
 		}
 		break;
 
--- a/source.html	Fri May 13 14:32:48 2011 -0600
+++ b/source.html	Fri May 13 15:42:04 2011 -0600
@@ -1799,14 +1799,15 @@
 
     <a href=http://example.com><b>A<a href=http://example.org>b</a>c</b></a>
 
-  IE 9 RC and Opera 11 produce simply:
+  (This doesn't round-trip through text/html serialization.)  IE 9 RC and Opera
+  11 produce simply:
 
     <a href=http://example.org><b>Abc</b></a>
 
-  The last behavior produces valid markup (unlike Gecko), is simple (unlike
-  WebKit), and probably best matches user expectations.  If you happen to miss
-  out a character when selecting the link you want to change, do you really
-  intend to only change the link of part of it? -->
+  The last behavior probably best matches user expectations.  If you happen to
+  miss out a character when selecting the link you want to change, do you
+  really intend to only change the link of part of it?
+  -->
 
   <li><span>Set the value</span> of each [[node]] in <var>node list</var> to
   <var>value</var>.
@@ -3621,10 +3622,23 @@
 
 <dt><code title><dfn title=command-unlink>unlink</dfn></code>
 
-<dd><strong>Action</strong>: <span>Decompose</span> the [[range]], and
-<span>set the value</span> of each returned [[node]] to null.
-<!-- IE 9 RC unlinks the whole link you're pointing at.  We follow the other
-browsers, which operate more consistently with other commands. -->
+<dd><strong>Action</strong>:
+<!--
+IE 9 RC unlinks the whole link you're pointing at, while others only
+unlink the current text.  The latter behavior seems less expected, as with
+createLink, although I can't articulate precisely why.  Word 2007 and
+OpenOffice.org 3.2.1 (Ubuntu) seem to give an option to remove the whole link
+or none of it, which backs the spec's requirement.  See also #whatwg logs
+starting at 2011-05-13 at 16:53 EDT (UTC-0400).
+-->
+
+<ol>
+  <li>Let <var>hyperlinks</var> be a list of every [[a]] element that has an
+  [[href]] attribute and is [[contained]] in the [[range]] or is an
+  [[ancestor]] of one of its [[boundarypoints]].
+
+  <li><span>Clear the value</span> of each member of <var>hyperlinks</var>.
+</ol>
 
 <dd><strong>State</strong>: Always false.