[webkit-reviews] review denied: [Bug 16749] DOMRange.surroundContents throws wrong exception (Acid3 bug) : [Attachment 18294] Proposed patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Andrew Wellington
<proton at wiretapped.net>'s request for review:
Bug 16749: DOMRange.surroundContents throws wrong exception (Acid3 bug)
http://bugs.webkit.org/show_bug.cgi?id=16749

Attachment 18294: Proposed patch
http://bugs.webkit.org/attachment.cgi?id=18294&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Looks good!

+    } else if (m_startContainer->nodeType() != Node::TEXT_NODE) {

I would just have used isTextNode() here rather than nodeType().

+	 if (m_startOffset > 0) {
+	     ec = RangeException::BAD_BOUNDARYPOINTS_ERR;
+	     return;
+	 }

This seems wrong. If m_startOffset is >= m_startContainer->maxCharacterOffset,
I think we're fine here.

review- because of that incorrect case. We need to add a test where the start
container is a comment node and the start offset is the end of the comment
node.