Range.isPointInRange should check whether the root nodes of boundary point and refNode are the same and return false if they are different before verifying the offset. See https://dom.spec.whatwg.org/#dom-range-ispointinrange This bug was found by the newly added LayoutTests/http/tests/w3c/dom/ranges/Range-isPointInRange.html.
<rdar://problem/22571620>
Created attachment 260590 [details] Fixes the bug
Created attachment 260591 [details] Fixed typos
Comment on attachment 260591 [details] Fixed typos View in context: https://bugs.webkit.org/attachment.cgi?id=260591&action=review > Source/WebCore/dom/Range.cpp:246 > + ExceptionCode ecToBeSupressed = 0; Spelling error here: The word suppressed has two "p" in it. I also think it’s a little strange to do this so differently here than above. Above we just use ec and set it to zero afterward. > Source/WebCore/dom/Range.cpp:-242 > - return compareBoundaryPoints(refNode, offset, &startContainer(), m_start.offset(), ec) >= 0 && !ec > - && compareBoundaryPoints(refNode, offset, &endContainer(), m_end.offset(), ec) <= 0 && !ec; How about writing it like this? bool result = compareBoundaryPoints(refNode, offset, &startContainer(), m_start.offset(), ec) >= 0 && !ec && compareBoundaryPoints(refNode, offset, &endContainer(), m_end.offset(), ec) <= 0 && !ec; ASSERT(!ec || ec == WRONG_DOCUMENT_ERR); ec = 0; return result; I think it’s simpler.
Comment on attachment 260591 [details] Fixed typos View in context: https://bugs.webkit.org/attachment.cgi?id=260591&action=review >> Source/WebCore/dom/Range.cpp:246 >> + ExceptionCode ecToBeSupressed = 0; > > Spelling error here: The word suppressed has two "p" in it. > > I also think it’s a little strange to do this so differently here than above. Above we just use ec and set it to zero afterward. Fixed. >> Source/WebCore/dom/Range.cpp:-242 >> - && compareBoundaryPoints(refNode, offset, &endContainer(), m_end.offset(), ec) <= 0 && !ec; > > How about writing it like this? > > bool result = compareBoundaryPoints(refNode, offset, &startContainer(), m_start.offset(), ec) >= 0 && !ec > && compareBoundaryPoints(refNode, offset, &endContainer(), m_end.offset(), ec) <= 0 && !ec; > ASSERT(!ec || ec == WRONG_DOCUMENT_ERR); > ec = 0; > return result; > > I think it’s simpler. Much cleaner! Done that.
Committed r189347: <http://trac.webkit.org/changeset/189347>