[Webkit-unassigned] [Bug 25571] DOM range methods setStartBefore/setEndAfter fails on detached elements

Mon Jan 27 16:02:25 PST 2014

https://bugs.webkit.org/show_bug.cgi?id=25571

Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #222309|review?                     |review-
               Flag|                            |

--- Comment #12 from Ryosuke Niwa <rniwa at webkit.org>  2014-01-27 15:59:48 PST ---
(From update of attachment 222309)
View in context: https://bugs.webkit.org/attachment.cgi?id=222309&action=review

> Source/WebCore/ChangeLog:9
> +        Range methods setStartBefore, setStartAfter, setEndBefore, setEndAfter throwing InvalidNodeTypeError exception
> +        since on detached node, root container of the node is not an Attr, Document, or DocumentFragment node.

Why are we making this behavioral change?  Is there any specification that mandates this behavior?
If so, please refer to the section of the specification in a specific revision (so that we'll have a reference even if the latest spec has changed).
r- due to the lack of justification.

> Source/WebCore/dom/Range.cpp:1214
> +        case Node::ELEMENT_NODE:

Please update or remove the comment at the beginning of this function.

> LayoutTests/fast/dom/Range/range-detachedNode.html:6
> +<head>
> +<title>Test for Range methods setStartBefore, setStartAfter, setEndBefore, setEndAfter on detached node</title>
> +<script src="../../../resources/js-test-pre.js"></script>
> +</head>

We don't need title or head. Just include js-test-pre.js in the body.

> LayoutTests/fast/dom/Range/range-detachedNode.html:15
> +var elm = document.getElementById('element');
> +function test() {
> +    // clone node, detaches the DOM
> +    elm = elm.cloneNode(true);

Please don't use abbreviations like elem. Spell out element.

> LayoutTests/fast/dom/Range/range-detachedNode.html:20
> +    shouldNotThrow('range.setStartBefore(elm.firstChild)');
> +
> +    shouldThrow('range.setStartBefore(elm)', '"Error: NotFoundError: DOM Exception 8"');
> +

We should also check that range.setStartBefore is setting the startContainer and startOffset correctly.
r- due to the inadequate testing.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.