Changeset 83081 in webkit

Timestamp:

Apr 6, 2011 12:24:20 PM (13 years ago)

Author:

commit-queue@webkit.org

Message:

2011-04-06 Robert Sesek <​rsesek@chromium.org>

Reviewed by Alexey Proskuryakov.

Move code duplicated between the WebKit/mac and WebKit2 down to WebCore because Chromium will need it too
​https://bugs.webkit.org/show_bug.cgi?id=54969

No change in behavior; no new tests.

• editing/TextIterator.cpp: (WebCore::TextIterator::locationAndLengthFromRange): New method from duplicated code in WebKits
• editing/TextIterator.h:
• page/Frame.cpp: (WebCore::Frame::rangeForPoint): New method from duplicated code in WebKits

2011-04-06 Robert Sesek <​rsesek@chromium.org>

Reviewed by Alexey Proskuryakov.

Move code duplicated between the WebKit/mac and WebKit2 down to WebCore because Chromium will need it too
​https://bugs.webkit.org/show_bug.cgi?id=54969

• WebView/WebFrame.mm: (-[WebFrame _convertToNSRange:]): Moved duplicated code to WebCore (-[WebFrame _characterRangeAtPoint:]): Moved duplicated code to WebCore

2011-04-06 Robert Sesek <​rsesek@chromium.org>

Reviewed by Alexey Proskuryakov.

Move code duplicated between the WebKit/mac and WebKit2 down to WebCore because Chromium will need it too
​https://bugs.webkit.org/show_bug.cgi?id=54969

• WebProcess/WebCoreSupport/WebEditorClient.cpp: (WebKit::WebEditorClient::respondToChangedSelection): Moved duplicated code to WebCore
• WebProcess/WebPage/mac/WebPageMac.mm: Moved duplicated code to WebCore (WebKit::WebPage::getMarkedRange): (WebKit::WebPage::getSelectedRange): (WebKit::WebPage::characterIndexForPoint): (WebKit::WebPage::performDictionaryLookupAtLocation):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/WebCore.exp.in (modified) (2 diffs)
• WebCore/editing/TextIterator.cpp (modified) (2 diffs)
• WebCore/editing/TextIterator.h (modified) (1 diff)
• WebCore/page/Frame.cpp (modified) (1 diff)
• WebCore/page/Frame.h (modified) (1 diff)
• WebKit/mac/ChangeLog (modified) (1 diff)
• WebKit/mac/WebView/WebFrame.mm (modified) (2 diffs)
• WebKit2/ChangeLog (modified) (1 diff)
• WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp (modified) (2 diffs)
• WebKit2/WebProcess/WebPage/WebPage.cpp (modified) (1 diff)
• WebKit2/WebProcess/WebPage/WebPage.h (modified) (1 diff)
• WebKit2/WebProcess/WebPage/mac/WebPageMac.mm (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-04-06  Robert Sesek  <rsesek@chromium.org>
+
+        Reviewed by Alexey Proskuryakov.
+
+        Move code duplicated between the WebKit/mac and WebKit2 down to WebCore because Chromium will need it too
+        https://bugs.webkit.org/show_bug.cgi?id=54969
+
+        No change in behavior; no new tests.
+
+        * editing/TextIterator.cpp:
+        (WebCore::TextIterator::locationAndLengthFromRange): New method from duplicated code in WebKits
+        * editing/TextIterator.h:
+        * page/Frame.cpp:
+        (WebCore::Frame::rangeForPoint): New method from duplicated code in WebKits
+
 2011-04-06  Leandro Gracia Gil  <leandrogracia@chromium.org>
 

trunk/Source/WebCore/WebCore.exp.in

@@ -299 +299 @@
 __ZN7WebCore12TextIterator11rangeLengthEPKNS_5RangeEb
 __ZN7WebCore12TextIterator26rangeFromLocationAndLengthEPNS_7ElementEiib
+__ZN7WebCore12TextIterator26locationAndLengthFromRangeEPKNS_5RangeERmS4_
 __ZN7WebCore12TextIterator7advanceEv
 __ZN7WebCore12TextIterator8subrangeEPNS_5RangeEii
@@ -721 +722 @@
 __ZN7WebCore5Frame9nodeImageEPNS_4NodeE
 __ZN7WebCore5Frame9scalePageEfRKNS_8IntPointE
+__ZN7WebCore5Frame13rangeForPointERKNS_8IntPointE
 __ZN7WebCore5FrameD1Ev
 __ZN7WebCore5Image12supportsTypeERKN3WTF6StringE

trunk/Source/WebCore/editing/TextIterator.cpp

@@ -29 +29 @@
 
 #include "Document.h"
+#include "Frame.h"
 #include "HTMLElement.h"
 #include "HTMLNames.h"
@@ -2370 +2371 @@
 }
 
+bool TextIterator::locationAndLengthFromRange(const Range* range, size_t& location, size_t& length)
+{
+    location = notFound;
+    length = 0;
+
+    if (!range->startContainer())
+        return false;
+
+    Element* selectionRoot = range->ownerDocument()->frame()->selection()->rootEditableElement();
+    Element* scope = selectionRoot ? selectionRoot : range->ownerDocument()->documentElement();
+
+    // The critical assumption is that this only gets called with ranges that
+    // concentrate on a given area containing the selection root. This is done
+    // because of text fields and textareas. The DOM for those is not
+    // directly in the document DOM, so ensure that the range does not cross a
+    // boundary of one of those.
+    if (range->startContainer() != scope && !range->startContainer()->isDescendantOf(scope))
+        return false;
+    if (range->endContainer() != scope && !range->endContainer()->isDescendantOf(scope))
+        return false;
+
+    RefPtr<Range> testRange = Range::create(scope->document(), scope, 0, range->startContainer(), range->startOffset());
+    ASSERT(testRange->startContainer() == scope);
+    location = TextIterator::rangeLength(testRange.get());
+    
+    ExceptionCode ec;
+    testRange->setEnd(range->endContainer(), range->endOffset(), ec);
+    ASSERT(testRange->startContainer() == scope);
+    length = TextIterator::rangeLength(testRange.get()) - location;
+    return true;
+}
+
 // --------
     

trunk/Source/WebCore/editing/TextIterator.h

@@ -99 +99 @@
     static int rangeLength(const Range*, bool spacesForReplacedElements = false);
     static PassRefPtr<Range> rangeFromLocationAndLength(Element* scope, int rangeLocation, int rangeLength, bool spacesForReplacedElements = false);
+    static bool locationAndLengthFromRange(const Range*, size_t& location, size_t& length);
     static PassRefPtr<Range> subrange(Range* entireRange, int characterOffset, int characterCount);
     

trunk/Source/WebCore/page/Frame.cpp

@@ -805 +805 @@
 }
 
+PassRefPtr<Range> Frame::rangeForPoint(const IntPoint& framePoint)
+{
+    VisiblePosition position = visiblePositionForPoint(framePoint);
+    if (position.isNull())
+        return 0;
+
+    VisiblePosition previous = position.previous();
+    if (previous.isNotNull()) {
+        RefPtr<Range> previousCharacterRange = makeRange(previous, position);
+        IntRect rect = editor()->firstRectForRange(previousCharacterRange.get());
+        if (rect.contains(framePoint))
+            return previousCharacterRange.release();
+    }
+
+    VisiblePosition next = position.next();
+    if (next.isNotNull()) {
+        RefPtr<Range> nextCharacterRange = makeRange(position, next);
+        IntRect rect = editor()->firstRectForRange(nextCharacterRange.get());
+        if (rect.contains(framePoint))
+            return nextCharacterRange.release();
+    }
+
+    return 0;
+}
+
 void Frame::createView(const IntSize& viewportSize,
                        const Color& backgroundColor, bool transparent,

trunk/Source/WebCore/page/Frame.h

@@ -185 +185 @@
         VisiblePosition visiblePositionForPoint(const IntPoint& framePoint);
         Document* documentAtPoint(const IntPoint& windowPoint);
+        PassRefPtr<Range> rangeForPoint(const IntPoint& framePoint);
 
         String searchForLabelsAboveCell(RegularExpression*, HTMLTableCellElement*, size_t* resultDistanceFromStartOfCell);

trunk/Source/WebKit/mac/ChangeLog

@@ +1 @@
+2011-04-06  Robert Sesek  <rsesek@chromium.org>
+
+        Reviewed by Alexey Proskuryakov.
+
+        Move code duplicated between the WebKit/mac and WebKit2 down to WebCore because Chromium will need it too
+        https://bugs.webkit.org/show_bug.cgi?id=54969
+
+        * WebView/WebFrame.mm:
+        (-[WebFrame _convertToNSRange:]): Moved duplicated code to WebCore
+        (-[WebFrame _characterRangeAtPoint:]): Moved duplicated code to WebCore
+
 2011-04-05  Alexey Proskuryakov  <ap@apple.com>
 

trunk/Source/WebKit/mac/WebView/WebFrame.mm

@@ -671 +671 @@
 - (NSRange)_convertToNSRange:(Range *)range
 {
-    if (!range || !range->startContainer())
+    if (!range)
         return NSMakeRange(NSNotFound, 0);
 
-    Element* selectionRoot = _private->coreFrame->selection()->rootEditableElement();
-    Element* scope = selectionRoot ? selectionRoot : _private->coreFrame->document()->documentElement();
-    
-    // Mouse events may cause TSM to attempt to create an NSRange for a portion of the view
-    // that is not inside the current editable region.  These checks ensure we don't produce
-    // potentially invalid data when responding to such requests.
-    if (range->startContainer() != scope && !range->startContainer()->isDescendantOf(scope))
+    size_t location;
+    size_t length;
+    if (!TextIterator::locationAndLengthFromRange(range, location, length))
         return NSMakeRange(NSNotFound, 0);
-    if (range->endContainer() != scope && !range->endContainer()->isDescendantOf(scope))
-        return NSMakeRange(NSNotFound, 0);
-    
-    RefPtr<Range> testRange = Range::create(scope->document(), scope, 0, range->startContainer(), range->startOffset());
-    ASSERT(testRange->startContainer() == scope);
-    int startPosition = TextIterator::rangeLength(testRange.get());
-
-    ExceptionCode ec;
-    testRange->setEnd(range->endContainer(), range->endOffset(), ec);
-    ASSERT(testRange->startContainer() == scope);
-    int endPosition = TextIterator::rangeLength(testRange.get());
-
-    return NSMakeRange(startPosition, endPosition - startPosition);
+
+    return NSMakeRange(location, length);
 }
 
@@ -821 +806 @@
 - (DOMRange *)_characterRangeAtPoint:(NSPoint)point
 {
-    VisiblePosition position = [self _visiblePositionForPoint:point];
-    if (position.isNull())
-        return nil;
-    
-    VisiblePosition previous = position.previous();
-    if (previous.isNotNull()) {
-        DOMRange *previousCharacterRange = kit(makeRange(previous, position).get());
-        NSRect rect = [self _firstRectForDOMRange:previousCharacterRange];
-        if (NSPointInRect(point, rect))
-            return previousCharacterRange;
-    }
-
-    VisiblePosition next = position.next();
-    if (next.isNotNull()) {
-        DOMRange *nextCharacterRange = kit(makeRange(position, next).get());
-        NSRect rect = [self _firstRectForDOMRange:nextCharacterRange];
-        if (NSPointInRect(point, rect))
-            return nextCharacterRange;
-    }
-    
-    return nil;
+    return kit(_private->coreFrame->rangeForPoint(IntPoint(point)).get());
 }
 

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2011-04-06  Robert Sesek  <rsesek@chromium.org>
+
+        Reviewed by Alexey Proskuryakov.
+
+        Move code duplicated between the WebKit/mac and WebKit2 down to WebCore because Chromium will need it too
+        https://bugs.webkit.org/show_bug.cgi?id=54969
+
+        * WebProcess/WebCoreSupport/WebEditorClient.cpp:
+        (WebKit::WebEditorClient::respondToChangedSelection): Moved duplicated code to WebCore
+        * WebProcess/WebPage/mac/WebPageMac.mm: Moved duplicated code to WebCore
+        (WebKit::WebPage::getMarkedRange):
+        (WebKit::WebPage::getSelectedRange):
+        (WebKit::WebPage::characterIndexForPoint):
+        (WebKit::WebPage::performDictionaryLookupAtLocation):
+
 2011-04-06  Chang Shu  <cshu@webkit.org>
 

trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp

@@ -44 +44 @@
 #include <WebCore/NotImplemented.h>
 #include <WebCore/Page.h>
+#include <WebCore/TextIterator.h>
 #include <WebCore/UserTypingGestureIndicator.h>
 
@@ -197 +198 @@
     selectionState.hasComposition = frame->editor()->hasComposition();
 
-    WebPage::getLocationAndLengthFromRange(frame->selection()->toNormalizedRange().get(), selectionState.selectedRangeStart, selectionState.selectedRangeLength);
+    Range* range = frame->selection()->toNormalizedRange().get();
+    if (range) {
+        size_t location;
+        size_t length;
+        if (!TextIterator::locationAndLengthFromRange(range, location, length))
+            return;
+        selectionState.selectedRangeStart = static_cast<uint64_t>(location);
+        selectionState.selectedRangeLength = static_cast<uint64_t>(length);
+    }
 
     m_page->send(Messages::WebPageProxy::SelectionStateChanged(selectionState));

trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp

@@ -825 +825 @@
 }
 
-void WebPage::getLocationAndLengthFromRange(Range* range, uint64_t& location, uint64_t& length)
-{
-    location = notFound;
-    length = 0;
-
-    if (!range || !range->startContainer())
-        return;
-
-    Element* selectionRoot = range->ownerDocument()->frame()->selection()->rootEditableElement();
-    Element* scope = selectionRoot ? selectionRoot : range->ownerDocument()->documentElement();
-    
-    // Mouse events may cause TSM to attempt to create an NSRange for a portion of the view
-    // that is not inside the current editable region.  These checks ensure we don't produce
-    // potentially invalid data when responding to such requests.
-    if (range->startContainer() != scope && !range->startContainer()->isDescendantOf(scope))
-        return;
-    if (range->endContainer() != scope && !range->endContainer()->isDescendantOf(scope))
-        return;
-
-    RefPtr<Range> testRange = Range::create(scope->document(), scope, 0, range->startContainer(), range->startOffset());
-    ASSERT(testRange->startContainer() == scope);
-    location = TextIterator::rangeLength(testRange.get());
-    
-    ExceptionCode ec;
-    testRange->setEnd(range->endContainer(), range->endOffset(), ec);
-    ASSERT(testRange->startContainer() == scope);
-    length = TextIterator::rangeLength(testRange.get()) - location;
-}
-
 // Events 
 

trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h

@@ -301 +301 @@
     SandboxExtensionTracker& sandboxExtensionTracker() { return m_sandboxExtensionTracker; }
 
-    static void getLocationAndLengthFromRange(WebCore::Range*, uint64_t& location, uint64_t& length);
-
 #if PLATFORM(MAC)
     void registerUIProcessAccessibilityTokens(const CoreIPC::DataReference& elemenToken, const CoreIPC::DataReference& windowToken);

trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm

@@ -277 +277 @@
     if (!frame)
         return;
-    
-    getLocationAndLengthFromRange(frame->editor()->compositionRange().get(), location, length);
+
+    Range* range = frame->editor()->compositionRange().get();
+    size_t locationSize;
+    size_t lengthSize;
+    if (range && TextIterator::locationAndLengthFromRange(range, locationSize, lengthSize)) {
+        location = static_cast<uint64_t>(locationSize);
+        length = static_cast<uint64_t>(lengthSize);
+    }
 }
 
@@ -288 +294 @@
     if (!frame)
         return;
-    
-    getLocationAndLengthFromRange(frame->selection()->toNormalizedRange().get(), location, length);
-}
-
-static PassRefPtr<Range> characterRangeAtPositionForPoint(Frame* frame, const VisiblePosition& position, const IntPoint& point)
-{
-    if (position.isNull())
-        return 0;
-
-    VisiblePosition previous = position.previous();
-    if (previous.isNotNull()) {
-        RefPtr<Range> previousCharacterRange = makeRange(previous, position);
-        IntRect rect = frame->editor()->firstRectForRange(previousCharacterRange.get());
-        if (rect.contains(point))
-            return previousCharacterRange.release();
-    }
-    
-    VisiblePosition next = position.next();
-    if (next.isNotNull()) {
-        RefPtr<Range> nextCharacterRange = makeRange(position, next);
-        IntRect rect = frame->editor()->firstRectForRange(nextCharacterRange.get());
-        if (rect.contains(point))
-            return nextCharacterRange.release();
-    }
-    
-    return 0;
-}
-
-static PassRefPtr<Range> characterRangeAtPoint(Frame* frame, const IntPoint& point)
-{
-    return characterRangeAtPositionForPoint(frame, frame->visiblePositionForPoint(point), point);
+
+    size_t locationSize;
+    size_t lengthSize;
+    if (TextIterator::locationAndLengthFromRange(frame->selection()->toNormalizedRange().get(), locationSize, lengthSize)) {
+        location = static_cast<uint64_t>(locationSize);
+        length = static_cast<uint64_t>(lengthSize);
+    }
 }
 
@@ -331 +313 @@
     frame = result.innerNonSharedNode() ? result.innerNonSharedNode()->document()->frame() : m_page->focusController()->focusedOrMainFrame();
     
-    RefPtr<Range> range = characterRangeAtPoint(frame, result.point());
+    RefPtr<Range> range = frame->rangeForPoint(result.point());
     if (!range)
         return;
 
-    uint64_t length;
-    getLocationAndLengthFromRange(range.get(), index, length);
+    size_t location;
+    size_t length;
+    if (TextIterator::locationAndLengthFromRange(range.get(), location, length))
+        index = static_cast<uint64_t>(location);
 }
 
@@ -421 +405 @@
 
     IntPoint translatedPoint = frame->view()->windowToContents(point);
+
+    // Don't do anything if there is no character at the point.
+    if (!frame->rangeForPoint(translatedPoint))
+        return;
+
     VisiblePosition position = frame->visiblePositionForPoint(translatedPoint);
-
-    // Don't do anything if there is no character at the point.
-    if (!characterRangeAtPositionForPoint(frame, position, translatedPoint))
-        return;
-
     VisibleSelection selection = m_page->focusController()->focusedOrMainFrame()->selection()->selection();
     if (shouldUseSelection(position, selection)) {