Skip to content
Permalink
Browse files
2010-04-01 Daniel Bates <dbates@rim.com>
        Reviewed by Darin Adler.

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

        Removed functions Range::operator == and Range::operator != as they
        were using C++ code that was not sound and hence may have undefined
        behavior.

        Test case: manual-tests/crash-on-find-with-no-selection.html

        * dom/Range.cpp:
        (WebCore::areRangesEqual): Added.
        * dom/Range.h:
        * editing/markup.cpp:
        (WebCore::createMarkup): Modified to call WebCore::areRangesEqual.
        * manual-tests/crash-on-find-with-no-selection.html: Added.
        * page/Frame.cpp:
        (WebCore::Frame::findString): Modified to call WebCore::areRangesEqual.


Canonical link: https://commits.webkit.org/48233@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@56940 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
dydz committed Apr 1, 2010
1 parent d32df19 commit ac2332e5b1e53d474b2342ed2f26e8dbd9042195
Showing 6 changed files with 40 additions and 10 deletions.
@@ -1,3 +1,24 @@
2010-04-01 Daniel Bates <dbates@rim.com>

Reviewed by Darin Adler.

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

Removed functions Range::operator == and Range::operator != as they
were using C++ code that was not sound and hence may have undefined
behavior.

Test case: manual-tests/crash-on-find-with-no-selection.html

* dom/Range.cpp:
(WebCore::areRangesEqual): Added.
* dom/Range.h:
* editing/markup.cpp:
(WebCore::createMarkup): Modified to call WebCore::areRangesEqual.
* manual-tests/crash-on-find-with-no-selection.html: Added.
* page/Frame.cpp:
(WebCore::Frame::findString): Modified to call WebCore::areRangesEqual.

2010-04-01 Geoffrey Garen <ggaren@apple.com>

Reviewed by Sam Weinig.
@@ -1661,15 +1661,13 @@ void Range::formatForDebugger(char* buffer, unsigned length) const
#undef FormatBufferSize
#endif

bool operator==(const Range& a, const Range& b)
bool areRangesEqual(const Range* a, const Range* b)
{
if (&a == &b)
if (a == b)
return true;
// Not strictly legal C++, but in practice this can happen, and this check works
// fine with GCC to detect such cases and return false rather than crashing.
if (!&a || !&b)
if (!a || !b)
return false;
return a.startPosition() == b.startPosition() && a.endPosition() == b.endPosition();
return a->startPosition() == b->startPosition() && a->endPosition() == b->endPosition();
}

PassRefPtr<Range> rangeOfContents(Node* node)
@@ -153,8 +153,7 @@ class Range : public RefCounted<Range> {

PassRefPtr<Range> rangeOfContents(Node*);

bool operator==(const Range&, const Range&);
inline bool operator!=(const Range& a, const Range& b) { return !(a == b); }
bool areRangesEqual(const Range*, const Range*);

} // namespace

@@ -970,7 +970,7 @@ String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterc

Node* body = enclosingNodeWithTag(Position(commonAncestor, 0), bodyTag);
// FIXME: Do this for all fully selected blocks, not just the body.
Node* fullySelectedRoot = body && *VisibleSelection::selectionFromContentsOfNode(body).toNormalizedRange() == *updatedRange ? body : 0;
Node* fullySelectedRoot = body && areRangesEqual(VisibleSelection::selectionFromContentsOfNode(body).toNormalizedRange().get(), updatedRange.get()) ? body : 0;
RefPtr<CSSMutableStyleDeclaration> fullySelectedRootStyle = fullySelectedRoot ? styleFromMatchedRulesAndInlineDecl(fullySelectedRoot) : 0;
if (annotate && fullySelectedRoot) {
if (shouldIncludeWrapperForFullySelectedRoot(fullySelectedRoot, fullySelectedRootStyle.get()))
@@ -0,0 +1,12 @@
<html>
<head>
</head>
<body>
<p>This test can be used to verify that we do not crash when searching for text on a page when nothing on the page is currently selected.</p>
<ol>
<li>Ensure that you have not clicked anywhere on this page and that nothing on this page is currently selected.</li>
<li>Search for the word &quot;crash&quot; in this page (In Safari for Mac, select Edit->Find->Find or press Cmd-F on your keyboard to open the Find banner to search for a word in the page).
<li>This test PASSED if we do not crash and the word &quot;crash&quot; is highlighted in at least the first sentence on this page.</li>
</ol>
</body>
</html>
@@ -1386,7 +1386,7 @@ bool Frame::findString(const String& target, bool forward, bool caseFlag, bool w
// If we started in the selection and the found range exactly matches the existing selection, find again.
// Build a selection with the found range to remove collapsed whitespace.
// Compare ranges instead of selection objects to ignore the way that the current selection was made.
if (startInSelection && *VisibleSelection(resultRange.get()).toNormalizedRange() == *selection.toNormalizedRange()) {
if (startInSelection && areRangesEqual(VisibleSelection(resultRange.get()).toNormalizedRange().get(), selection.toNormalizedRange().get())) {
searchRange = rangeOfContents(document());
if (forward)
setStart(searchRange.get(), selection.visibleEnd());

0 comments on commit ac2332e

Please sign in to comment.