Bug 19968 - splitText() can create ranges with end before start
splitText() can create ranges with end before start
Status: RESOLVED FIXED
Product: WebAppsWG
Classification: Unclassified
Component: DOM
unspecified
All All
: P2 normal
: ---
Assigned To: Anne
public-webapps-bugzilla
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-15 12:50 UTC by Aryeh Gregor
Modified: 2012-11-25 12:02 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Aryeh Gregor 2012-11-15 12:50:32 UTC
Test-case:

data:text/html,<!doctype html>
abcdef<br>
<script>
document.body.firstChild.splitText(3);
var range = document.createRange();
range.setStart(document.body.firstChild, 2);
range.setEnd(document.body, 1);
document.body.firstChild.splitText(1);
document.body.appendChild(document.createTextNode(
"(" + range.startContainer.nodeName + " " + range.startContainer.nodeValue
+ ", " + range.startOffset
+ ") (" + range.endContainer.nodeName + " " + range.endContainer.nodeValue
+ ", " + range.endOffset + ")"));
</script>

Gecko and WebKit give "(#text bc, 1) (BODY null, 1)" -- where the node "#text bc" is the second child of <body>, so the end is before the start.  Opera gives "(#text bc, 1) (#text bc, 1)", which makes no sense but at least isn't backwards.  IE gives "(#text bc, 2) (BODY null, 1)", which is the most broken of all.  The spec requires Gecko/WebKit behavior, and is thus obviously wrong.

Mats Palmgren explains the problem in detail in this Mozilla bug: https://bugzilla.mozilla.org/show_bug.cgi?id=803924#c19  He also gives a suggested fix, namely to have splitText() explicitly handle the case where a boundary point falls at the insertion point of the new node.  His fix looks good to me.
Comment 1 Mats Palmgren 2012-11-16 11:49:40 UTC
> IE gives "(#text bc, 2) (BODY null, 1)", which is the most
> broken of all.

I think the latest IE has been fixed.
The result I get with the preview version(*) of IE10 for Windows 7 is:
(#text bc, 1) (BODY null, 2) 

(*) IE10.0.9200.16438 to be exact

Furthermore, the subset of tests in test_Range-mutations.html that failed
with my fix in 803924 also fails in that IE version:

splitTextTests.push(
        ["paras[0].firstChild", 1, "paras[0]", 0, "paras[0]", 1],
        ["paras[0].firstChild", 1, "paras[0]", 1, "paras[0]", 1],
        ["paras[0].firstChild", 1, "paras[0].firstChild", 1, "paras[0]", 1],
        ["paras[0].firstChild", 2, "paras[0].firstChild", 1, "paras[0]", 1],
        ["paras[0].firstChild", 3, "paras[0].firstChild", 1, "paras[0]", 1],
        ["paras[0].firstChild", 1, "paras[0].firstChild", 2, "paras[0]", 1]
);

After I fixed the testSplitText() function in test_Range-mutations.html
to implement the proposed spec change, both IE and my patched Firefox
pass those tests.

I think this strengthens the argument for changing the spec in the way
I propose, since two UA vendors have *independently* implemented the
same fix.
Comment 3 Aryeh Gregor 2012-11-25 12:02:38 UTC
Tests updated:

http://dvcs.w3.org/hg/webapps/rev/7250c98632b2

Firefox nightly now passes all of them again.  (The old nightlies passed the old tests, then Firefox was fixed and failed some of the tests, now the tests are fixed so it passes again.)