REGRESSION(r74971): Selection doesn't work correctly in BiDi Text
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Sep 2011 18:30:29 +0000 (18:30 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Sep 2011 18:30:29 +0000 (18:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=57340

Reviewed by Eric Seidel.

Source/WebCore:

This patch adds the end point adjustment mechanism at bidi boundaries similar to the one NSTextView implements.

To understand the problem, suppose we have strong RTL letters "ABC" in a LTR block (visually laid out as CBA).

Per NSTextView convention, logical offsets between each letter is placed as (0)C(2)B(1)A(3). In other words,
placing the caret visually on the left of CBA yields the position inside the text node of "ABC" at offset 0.
Likewise, placing it between C and B yields ("ABC", 2), and placing it on the right of CBA yields ("ABC", 3).

Now suppose a user attempts to select the letter A by a mouse drag from the right of CBA to a point between
B and A. First, the initial mouse down places the selection's base at ("ABC", 3). Then as the mouse pointer
moves to a point on the left of A, the selection's extent is set at ("ABC", 1), selecting "BC".

To mitigate this issue, NSTextView adjusts selection base and extent under certain conditions. In the above
example, NSTextView detects user's intent and changes the selection's base to ("ABC", 0) temporarily.

This patch implements a similar trick on WebKit. We adjust the base or the extent when they're at the left
end or at the right end of a bidi run and the other end is inside of the run. In the above example, the
base position on the right of A is the right end of a bidi run and the extent position between B and A is
inside the same run (CBA), so we would adjust the base to be ("ABC", 0) as NSTextView does.

Take another example abcABC. Note offsets are assigned as (0)a(1)b(2)c(3)C(5)B(4)A(6) When the user starts
a mouse drag from the right of A to a point between B and A, we adjust the selection base to be ("abcABC", 3)
because the base is at the right end of a bidi run and the extent is in the same run. We keep the adjustment
when the mouse pointer moves to a point between C and B. However, when the mouser pointer reaches a point
between letters b and c, the selection extent is placed at ("abcABC", 2). Because the extent is outside of
the bidi run started from the selection base, we restore the original base at this point. Had we not done this,
we'll end up selecting just "c".

While this algorithm is implemented in FrameSelection::setNonDirectionalSelectionIfNeeded, this patch adds
various member functions to RenderedPosition to facilitate abstraction around inline boxes and bidi runs.

Test: editing/selection/select-bidi-run.html

* editing/FrameSelection.cpp:
(WebCore::adjustEndpointsAtBidiBoundary): Added. Implements the endpoints adjustment algorithm.
(WebCore::FrameSelection::setNonDirectionalSelectionIfNeeded): Calls adjustEndpointsAtBidiBoundary, and
restores the original base as needed.
* editing/FrameSelection.h:
* editing/RenderedPosition.cpp:
(WebCore::RenderedPosition::RenderedPosition):
(WebCore::RenderedPosition::prevLeafChild): Added to cache prevLeafChild of the current inline box.
(WebCore::RenderedPosition::nextLeafChild): Ditto for nextLeafChild.
(WebCore::RenderedPosition::isEquivalent): Compares two RenderedPositions considering neighboring inline boxes
so that the rightmost position in a box and the leftmost position in the following box is considered equal.
(WebCore::RenderedPosition::bidiLevelOnLeft): Added. Returns the bidi level of the run on the left. We can't
add a generic bidiLevel to this class because it'll be ambiguous at bidi boundaries.
(WebCore::RenderedPosition::bidiLevelOnRight): Ditto for the run on the right.
(WebCore::RenderedPosition::leftBoundaryOfBidiRun): Added.
(WebCore::RenderedPosition::rightBoundaryOfBidiRun): Added.
(WebCore::RenderedPosition::atLeftBoundaryOfBidiRun): Added.
(WebCore::RenderedPosition::atRightBoundaryOfBidiRun): Added.
(WebCore::RenderedPosition::positionAtLeftBoundaryOfBiDiRun): Returns Position at the left edge of a bidi run
if RenderedPosition is at such a position. Asserts atLeftBoundaryOfBidiRun.
(WebCore::RenderedPosition::positionAtRightBoundaryOfBiDiRun): Ditto for the right edge.
* editing/RenderedPosition.h:
(WebCore::RenderedPosition::atLeftBoundaryOfBidiRun): Added.
(WebCore::RenderedPosition::atRightBoundaryOfBidiRun): Added.
(WebCore::RenderedPosition::atLeftmostOffsetInBox): Added.
(WebCore::RenderedPosition::atRightmostOffsetInBox): Added.
(WebCore::RenderedPosition::uncachedInlineBox): Added. We can't use a static const variable because gcc thinks
reinterpret_cast<InlineBox*>(1) is not an integral value.
(WebCore::RenderedPosition::RenderedPosition):
* editing/VisibleSelection.h:
(WebCore::VisibleSelection::visibleBase): Added.
(WebCore::VisibleSelection::visibleExtent): Added.
* page/EventHandler.cpp:
(WebCore::EventHandler::updateSelectionForMouseDrag):

LayoutTests:

* editing/selection/select-bidi-run-expected.txt: Added.
* editing/selection/select-bidi-run.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@95964 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/editing/selection/select-bidi-run-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/select-bidi-run.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/FrameSelection.cpp
Source/WebCore/editing/FrameSelection.h
Source/WebCore/editing/RenderedPosition.cpp
Source/WebCore/editing/RenderedPosition.h
Source/WebCore/editing/VisibleSelection.h
Source/WebCore/page/EventHandler.cpp

index 471110d..31d8bae 100644 (file)
@@ -1,3 +1,13 @@
+2011-09-12  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION(r74971): Selection doesn't work correctly in BiDi Text
+        https://bugs.webkit.org/show_bug.cgi?id=57340
+
+        Reviewed by Eric Seidel.
+
+        * editing/selection/select-bidi-run-expected.txt: Added.
+        * editing/selection/select-bidi-run.html: Added.
+
 2011-09-26  Csaba Osztrogonác  <ossy@webkit.org>
 
         [Qt] Unreviewed gardening.
diff --git a/LayoutTests/editing/selection/select-bidi-run-expected.txt b/LayoutTests/editing/selection/select-bidi-run-expected.txt
new file mode 100644 (file)
index 0000000..4e876e9
--- /dev/null
@@ -0,0 +1,122 @@
+This test ensures WebKit lets user select bidirectional text intuitively. To manually test, select text in blue box in each test case below by a mouse drag from left to right. The changes in the selected text should match the expectations before |. Do the same by a mouse drag from right to left and expectations are after |.
+
+Test "abcABC" in "abcABC":
+Selecting from left to right
+PASS selected "a"
+PASS selected "ab"
+PASS selected "abc"
+PASS selected "abcAB"
+PASS selected "abcA"
+PASS selected "abcABC"
+Selecting from right to left
+PASS selected "A"
+PASS selected "AB"
+PASS selected "ABC"
+PASS selected "cABC"
+PASS selected "bcABC"
+PASS selected "abcABC"
+
+Test "ABCdef" in "ABCdef":
+Selecting from left to right
+PASS selected "C"
+PASS selected "BC"
+PASS selected "ABC"
+PASS selected "ABCd"
+FAIL selected "ABCde" but expected "ABCef"
+PASS selected "ABCdef"
+Selecting from right to left
+PASS selected "f"
+PASS selected "ef"
+PASS selected "def"
+PASS selected "BCdef"
+PASS selected "Cdef"
+PASS selected "ABCdef"
+
+Test "ABC" in "abcABCdef":
+Selecting from left to right
+PASS selected "C"
+PASS selected "BC"
+PASS selected "ABC"
+Selecting from right to left
+PASS selected "A"
+PASS selected "AB"
+PASS selected "ABC"
+
+Test "ABC" in "ABCdef":
+Selecting from left to right
+PASS selected "C"
+PASS selected "BC"
+PASS selected "ABC"
+Selecting from right to left
+PASS selected "A"
+PASS selected "AB"
+PASS selected "ABC"
+
+Test "ef" in "ABCdef":
+Selecting from left to right
+PASS selected "e"
+PASS selected "ef"
+Selecting from right to left
+PASS selected "f"
+PASS selected "ef"
+
+Test "AB" in "abcABC":
+Selecting from left to right
+PASS selected "B"
+PASS selected "AB"
+Selecting from right to left
+PASS selected "A"
+PASS selected "AB"
+
+Test "12" in "aXM12JNd":
+Selecting from left to right
+PASS selected "1"
+PASS selected "12"
+Selecting from right to left
+PASS selected "2"
+PASS selected "12"
+
+Test "ABC 123" in "ABC 123":
+Selecting from left to right
+PASS selected "1"
+PASS selected "12"
+PASS selected "123"
+PASS selected " 123"
+PASS selected "C 123"
+PASS selected "BC 123"
+PASS selected "ABC 123"
+Selecting from right to left
+PASS selected "A"
+PASS selected "AB"
+PASS selected "ABC"
+PASS selected "ABC "
+PASS selected "ABC 12"
+PASS selected "ABC 1"
+PASS selected "ABC 123"
+
+Test "ABC 123" in "ABC 123":
+Selecting from left to right
+PASS selected "1"
+PASS selected "12"
+FAIL selected "123" but expected " 123"
+FAIL selected " " but expected "C 123"
+FAIL selected "C " but expected "BC 123"
+FAIL selected "BC " but expected "ABC 123"
+FAIL selected "123" but expected "undefined"
+Selecting from right to left
+PASS selected "A"
+PASS selected "AB"
+PASS selected "ABC"
+FAIL selected "" but expected "ABC "
+PASS selected "ABC 12"
+PASS selected "ABC 1"
+FAIL selected "123" but expected "ABC 123"
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+
+
+
+
diff --git a/LayoutTests/editing/selection/select-bidi-run.html b/LayoutTests/editing/selection/select-bidi-run.html
new file mode 100644 (file)
index 0000000..78c5ea7
--- /dev/null
@@ -0,0 +1,141 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta http-equiv="content-type" content="text/html; charset=utf-8">
+<link rel="stylesheet" href="../../fast/js/resources/js-test-style.css">
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+<style type="text/css">
+#tests { font-size: 2.5em; padding: 0px; margin: 0px; }
+dt { width: 15ex; padding: 0px 10px; margin: 0px; }
+dd { font-size: 0.6em; margin: 0px; padding: 0px 10px; }
+.target { background-color: #bbeeff; }
+</style>
+</head>
+<body>
+<p>This test ensures WebKit lets user select bidirectional text intuitively.
+To manually test, select text in blue box in each test case below by a mouse drag from left to right.
+The changes in the selected text should match the expectations before |.
+Do the same by a mouse drag from right to left and expectations are after |.</p>
+<div>Selected text: <span id="log"></span></div>
+<dl id="tests">
+<dt contenteditable><span class="target">abcאבג</span></dt>
+<dd>a,ab,abc,abcAB,abcA,abcABC|A,AB,ABC,cABC,bcABC,abcABC</dd>
+
+<dt contenteditable><span class="target">אבגdef</span></dt>
+<dd>C,BC,ABC,ABCd,ABCef,ABCdef|f,ef,def,BCdef,Cdef,ABCdef</dd>
+
+<dt contenteditable>abc<span class="target">אבג</span>def</dt>
+<dd>C,BC,ABC|A,AB,ABC</dd>
+
+<dt dir="rtl" contenteditable><span class="target">אבג</span>def</dt>
+<dd>C,BC,ABC|A,AB,ABC</dd>
+
+<dt dir="rtl">אבגd<span class="target">ef</span></dt>
+<dd>e,ef|f,ef</dd>
+
+<dt contenteditable>abc<span class="target">אב</span>ג</dt>
+<dd>B,AB|A,AB</dd>
+
+<dt contenteditable>aקל<span class="target">12</span>יםd</dt>
+<dd>1,12|2,12</dd>
+
+<dt contenteditable dir="rtl"><span class="target">אבג 123</span></dt>
+<dd>1,12,123, 123,C 123,BC 123,ABC 123|A,AB,ABC,ABC ,ABC 12,ABC 1,ABC 123</dd>
+
+<dt contenteditable><span class="target">אבג 123</span></dt>
+<dd>1,12, 123,C 123,BC 123,ABC 123|A,AB,ABC,ABC ,ABC 12,ABC 1,ABC 123</dd>
+
+<!--<dt contenteditable><span class="target">אבג 123 - 456</span></dt>
+<dd>1,12, 123,C 123,BC 123,ABC 123|A,AB,ABC,ABC ,ABC 12,ABC 1,ABC 123</dd>-->
+
+</dl>
+<div id="console"></div>
+<pre><script>
+var tests = document.getElementById('tests');
+
+String.prototype.fold = function () {
+    var result = '';
+    for (var i = 0; i < this.length; i++) {
+        var code = this.charCodeAt(i);
+        if (0x05d0 <= code && code <= 0x05ea)// Hebrew Alef
+            code += -0x05d0 + 'A'.charCodeAt(0);
+        result += String.fromCharCode(code);
+    }
+    return result;
+}
+
+function assertEqual(expected, actual) {
+    document.writeln('E:' + expected + ' A:' + actual);
+}
+
+function selectByMouseDragAndVerifyResult(target, leftToRight, expectations) {
+    var y = target.offsetTop + target.offsetHeight / 2;
+    var left = target.offsetLeft;
+
+    var startX = left + (leftToRight ? 0 : target.offsetWidth);
+    eventSender.dragMode = false;
+    eventSender.leapForward(3000);
+    eventSender.mouseMoveTo(startX, y);
+    eventSender.mouseDown();
+
+    var previousSelectedText = '';
+    var j = 0;
+    var xIncrement = leftToRight ? 1 : -1;
+    for (var x = startX; left <= x && x <= left + target.offsetWidth; x += xIncrement) {
+        eventSender.leapForward(100);
+        eventSender.mouseMoveTo(x, y);
+
+        var selectedText = window.getSelection().toString().fold();
+        if (previousSelectedText != selectedText) {
+            if (expectations[j] == selectedText)
+                testPassed('selected "' + selectedText + '"');
+            else
+                testFailed('selected "' + selectedText + '" but expected "' + expectations[j] + '"');
+            previousSelectedText = selectedText;
+            j++;
+        }
+    }
+
+    eventSender.mouseUp();
+}
+
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+
+    var tests = document.getElementById('tests').getElementsByTagName('dt');
+    var testExpectations = document.getElementById('tests').getElementsByTagName('dd');
+
+    for (var i = 0; i < tests.length; i++) {        
+        tests[i].style.display = null;
+        testExpectations[i].style.display = null;
+
+        var target = tests[i].getElementsByClassName('target')[0];
+        var testExpectation = testExpectations[i].textContent;
+
+        debug('Test "' + target.textContent.fold() + '" in "' + target.parentNode.textContent.fold() + '":');
+        debug('Selecting from left to right');
+        selectByMouseDragAndVerifyResult(target, true, testExpectation.split(/\|/)[0].split(/,/));
+        debug('Selecting from right to left');
+        selectByMouseDragAndVerifyResult(target, false, testExpectation.split(/\|/)[1].split(/,/));
+        debug('');
+
+        // Some tests may be ouside of the window so bring them in as we process.
+        tests[i].style.display = 'none';
+        testExpectations[i].style.display = 'none';
+    }
+
+    document.getElementById('tests').style.display = 'none';
+    document.getElementById('log').parentNode.style.display = 'none';
+} else {
+    debug('This test requires eventSender');
+    document.onselectionchange = function () {
+        document.getElementById('log').textContent = window.getSelection().toString().fold();
+    }
+}
+
+var successfullyParsed = true;
+
+</script>
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
index dbcca4c..e03317b 100644 (file)
@@ -1,3 +1,78 @@
+2011-09-12  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION(r74971): Selection doesn't work correctly in BiDi Text
+        https://bugs.webkit.org/show_bug.cgi?id=57340
+
+        Reviewed by Eric Seidel.
+
+        This patch adds the end point adjustment mechanism at bidi boundaries similar to the one NSTextView implements.
+
+        To understand the problem, suppose we have strong RTL letters "ABC" in a LTR block (visually laid out as CBA).
+
+        Per NSTextView convention, logical offsets between each letter is placed as (0)C(2)B(1)A(3). In other words,
+        placing the caret visually on the left of CBA yields the position inside the text node of "ABC" at offset 0.
+        Likewise, placing it between C and B yields ("ABC", 2), and placing it on the right of CBA yields ("ABC", 3).
+
+        Now suppose a user attempts to select the letter A by a mouse drag from the right of CBA to a point between
+        B and A. First, the initial mouse down places the selection's base at ("ABC", 3). Then as the mouse pointer
+        moves to a point on the left of A, the selection's extent is set at ("ABC", 1), selecting "BC".
+
+        To mitigate this issue, NSTextView adjusts selection base and extent under certain conditions. In the above
+        example, NSTextView detects user's intent and changes the selection's base to ("ABC", 0) temporarily.
+
+        This patch implements a similar trick on WebKit. We adjust the base or the extent when they're at the left
+        end or at the right end of a bidi run and the other end is inside of the run. In the above example, the
+        base position on the right of A is the right end of a bidi run and the extent position between B and A is
+        inside the same run (CBA), so we would adjust the base to be ("ABC", 0) as NSTextView does.
+
+        Take another example abcABC. Note offsets are assigned as (0)a(1)b(2)c(3)C(5)B(4)A(6) When the user starts
+        a mouse drag from the right of A to a point between B and A, we adjust the selection base to be ("abcABC", 3)
+        because the base is at the right end of a bidi run and the extent is in the same run. We keep the adjustment
+        when the mouse pointer moves to a point between C and B. However, when the mouser pointer reaches a point
+        between letters b and c, the selection extent is placed at ("abcABC", 2). Because the extent is outside of
+        the bidi run started from the selection base, we restore the original base at this point. Had we not done this,
+        we'll end up selecting just "c".
+
+        While this algorithm is implemented in FrameSelection::setNonDirectionalSelectionIfNeeded, this patch adds
+        various member functions to RenderedPosition to facilitate abstraction around inline boxes and bidi runs.
+
+        Test: editing/selection/select-bidi-run.html
+
+        * editing/FrameSelection.cpp:
+        (WebCore::adjustEndpointsAtBidiBoundary): Added. Implements the endpoints adjustment algorithm.
+        (WebCore::FrameSelection::setNonDirectionalSelectionIfNeeded): Calls adjustEndpointsAtBidiBoundary, and
+        restores the original base as needed.
+        * editing/FrameSelection.h:
+        * editing/RenderedPosition.cpp:
+        (WebCore::RenderedPosition::RenderedPosition):
+        (WebCore::RenderedPosition::prevLeafChild): Added to cache prevLeafChild of the current inline box.
+        (WebCore::RenderedPosition::nextLeafChild): Ditto for nextLeafChild.
+        (WebCore::RenderedPosition::isEquivalent): Compares two RenderedPositions considering neighboring inline boxes
+        so that the rightmost position in a box and the leftmost position in the following box is considered equal.
+        (WebCore::RenderedPosition::bidiLevelOnLeft): Added. Returns the bidi level of the run on the left. We can't
+        add a generic bidiLevel to this class because it'll be ambiguous at bidi boundaries.
+        (WebCore::RenderedPosition::bidiLevelOnRight): Ditto for the run on the right.
+        (WebCore::RenderedPosition::leftBoundaryOfBidiRun): Added.
+        (WebCore::RenderedPosition::rightBoundaryOfBidiRun): Added.
+        (WebCore::RenderedPosition::atLeftBoundaryOfBidiRun): Added.
+        (WebCore::RenderedPosition::atRightBoundaryOfBidiRun): Added.
+        (WebCore::RenderedPosition::positionAtLeftBoundaryOfBiDiRun): Returns Position at the left edge of a bidi run
+        if RenderedPosition is at such a position. Asserts atLeftBoundaryOfBidiRun.
+        (WebCore::RenderedPosition::positionAtRightBoundaryOfBiDiRun): Ditto for the right edge.
+        * editing/RenderedPosition.h:
+        (WebCore::RenderedPosition::atLeftBoundaryOfBidiRun): Added.
+        (WebCore::RenderedPosition::atRightBoundaryOfBidiRun): Added.
+        (WebCore::RenderedPosition::atLeftmostOffsetInBox): Added.
+        (WebCore::RenderedPosition::atRightmostOffsetInBox): Added.
+        (WebCore::RenderedPosition::uncachedInlineBox): Added. We can't use a static const variable because gcc thinks
+        reinterpret_cast<InlineBox*>(1) is not an integral value.
+        (WebCore::RenderedPosition::RenderedPosition):
+        * editing/VisibleSelection.h:
+        (WebCore::VisibleSelection::visibleBase): Added.
+        (WebCore::VisibleSelection::visibleExtent): Added.
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::updateSelectionForMouseDrag):
+
 2011-09-26  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r95256.
index 0c58bf9..730d255 100644 (file)
@@ -56,6 +56,7 @@
 #include "RenderTheme.h"
 #include "RenderView.h"
 #include "RenderWidget.h"
+#include "RenderedPosition.h"
 #include "SecureTextInput.h"
 #include "Settings.h"
 #include "SpatialNavigation.h"
@@ -163,13 +164,66 @@ void DragCaretController::setCaretPosition(const VisiblePosition& position)
         updateCaretRect(document, m_position);
 }
 
-void FrameSelection::setNonDirectionalSelectionIfNeeded(const VisibleSelection& passedNewSelection, TextGranularity granularity)
+static void adjustEndpointsAtBidiBoundary(VisiblePosition& visibleBase, VisiblePosition& visibleExtent)
 {
-    VisibleSelection newSelection = passedNewSelection;
+    RenderedPosition base(visibleBase);
+    RenderedPosition extent(visibleExtent);
 
-    if (shouldAlwaysUseDirectionalSelection(m_frame))
-        newSelection.setIsDirectional(true);
+    if (base.isNull() || extent.isNull() || base.isEquivalent(extent))
+        return;
+
+    if (base.atLeftBoundaryOfBidiRun()) {
+        if (!extent.atRightBoundaryOfBidiRun(base.bidiLevelOnRight())
+            && base.isEquivalent(extent.leftBoundaryOfBidiRun(base.bidiLevelOnRight()))) {
+            visibleBase = base.positionAtLeftBoundaryOfBiDiRun();
+            return;
+        }
+        return;
+    }
+
+    if (base.atRightBoundaryOfBidiRun()) {
+        if (!extent.atLeftBoundaryOfBidiRun(base.bidiLevelOnLeft())
+            && base.isEquivalent(extent.rightBoundaryOfBidiRun(base.bidiLevelOnLeft()))) {
+            visibleBase = base.positionAtRightBoundaryOfBiDiRun();
+            return;
+        }
+        return;
+    }
+
+    if (extent.atLeftBoundaryOfBidiRun() && extent.isEquivalent(base.leftBoundaryOfBidiRun(extent.bidiLevelOnRight()))) {
+        visibleExtent = extent.positionAtLeftBoundaryOfBiDiRun();
+        return;
+    }
+
+    if (extent.atRightBoundaryOfBidiRun() && extent.isEquivalent(base.rightBoundaryOfBidiRun(extent.bidiLevelOnLeft()))) {
+        visibleExtent = extent.positionAtRightBoundaryOfBiDiRun();
+        return;
+    }
+}
+
+void FrameSelection::setNonDirectionalSelectionIfNeeded(const VisibleSelection& passedNewSelection, TextGranularity granularity,
+    EndPointsAdjustmentMode endpointsAdjustmentMode)
+{
+    VisibleSelection newSelection = passedNewSelection;
+    bool isDirectional = shouldAlwaysUseDirectionalSelection(m_frame) || newSelection.isDirectional();
+
+    VisiblePosition base = m_originalBase.isNotNull() ? m_originalBase : newSelection.visibleBase();
+    VisiblePosition newBase = base;
+    VisiblePosition newExtent = newSelection.visibleExtent();
+    if (endpointsAdjustmentMode == AdjustEndpointsAtBidiBoundary)
+        adjustEndpointsAtBidiBoundary(newBase, newExtent);
+
+    if (newBase != base || newExtent != newSelection.visibleExtent()) {
+        m_originalBase = base;
+        newSelection.setBase(newBase);
+        newSelection.setExtent(newExtent);
+    } else if (m_originalBase.isNotNull()) {
+        if (m_selection.base() == newSelection.base())
+            newSelection.setBase(m_originalBase);
+        m_originalBase.clear();
+    }
 
+    newSelection.setIsDirectional(isDirectional); // Adjusting base and extent will make newSelection always directional
     if (m_selection == newSelection || !shouldChangeSelection(newSelection))
         return;
 
index afe5d84..b26c474 100644 (file)
@@ -223,7 +223,8 @@ public:
 
     bool shouldChangeSelection(const VisibleSelection&) const;
     bool shouldDeleteSelection(const VisibleSelection&) const;
-    void setNonDirectionalSelectionIfNeeded(const VisibleSelection&, TextGranularity);
+    enum EndPointsAdjustmentMode { AdjustEndpointsAtBidiBoundary, DoNotAdjsutEndpoints };
+    void setNonDirectionalSelectionIfNeeded(const VisibleSelection&, TextGranularity, EndPointsAdjustmentMode = DoNotAdjsutEndpoints);
     void setFocusedNodeIfNeeded();
     void notifyRendererOfSelectionChange(EUserTriggered);
 
@@ -282,6 +283,7 @@ private:
     LayoutUnit m_xPosForVerticalArrowNavigation;
 
     VisibleSelection m_selection;
+    VisiblePosition m_originalBase; // Used to store base before the adjustment at bidi boundary
     TextGranularity m_granularity;
 
     RefPtr<EditingStyle> m_typingStyle;
index dd5e129..475f5a7 100644 (file)
@@ -32,6 +32,7 @@
 #include "RenderedPosition.h"
 
 #include "InlineBox.h"
+#include "InlineTextBox.h"
 #include "Position.h"
 #include "VisiblePosition.h"
 
@@ -68,6 +69,8 @@ RenderedPosition::RenderedPosition(const VisiblePosition& position)
     : m_renderer(0)
     , m_inlineBox(0)
     , m_offset(0)
+    , m_prevLeafChild(uncachedInlineBox())
+    , m_nextLeafChild(uncachedInlineBox())
 {
     if (position.isNull())
         return;
@@ -82,6 +85,8 @@ RenderedPosition::RenderedPosition(const Position& position, EAffinity affinity)
     : m_renderer(0)
     , m_inlineBox(0)
     , m_offset(0)
+    , m_prevLeafChild(uncachedInlineBox())
+    , m_nextLeafChild(uncachedInlineBox())
 {
     if (position.isNull())
         return;
@@ -92,6 +97,133 @@ RenderedPosition::RenderedPosition(const Position& position, EAffinity affinity)
         m_renderer = rendererFromPosition(position);
 }
 
+InlineBox* RenderedPosition::prevLeafChild() const
+{
+    if (m_prevLeafChild == uncachedInlineBox())
+        m_prevLeafChild = m_inlineBox->prevLeafChild();
+    return m_prevLeafChild;
+}
+
+InlineBox* RenderedPosition::nextLeafChild() const
+{
+    if (m_nextLeafChild == uncachedInlineBox())
+        m_nextLeafChild = m_inlineBox->nextLeafChild();
+    return m_nextLeafChild;
+}
+
+bool RenderedPosition::isEquivalent(const RenderedPosition& other) const
+{
+    return (m_renderer == other.m_renderer && m_inlineBox == other.m_inlineBox && m_offset == other.m_offset)
+        || (atLeftmostOffsetInBox() && other.atRightmostOffsetInBox() && prevLeafChild() == other.m_inlineBox)
+        || (atRightmostOffsetInBox() && other.atLeftmostOffsetInBox() && nextLeafChild() == other.m_inlineBox);
+}
+
+unsigned char RenderedPosition::bidiLevelOnLeft() const
+{
+    InlineBox* box = atLeftmostOffsetInBox() ? prevLeafChild() : m_inlineBox;
+    return box ? box->bidiLevel() : 0;
+}
+
+unsigned char RenderedPosition::bidiLevelOnRight() const
+{
+    InlineBox* box = atRightmostOffsetInBox() ? nextLeafChild() : m_inlineBox;
+    return box ? box->bidiLevel() : 0;
+}
+
+RenderedPosition RenderedPosition::leftBoundaryOfBidiRun(unsigned char bidiLevelOfRun)
+{
+    if (!m_inlineBox || bidiLevelOfRun > m_inlineBox->bidiLevel())
+        return RenderedPosition();
+
+    InlineBox* box = m_inlineBox;
+    do {
+        InlineBox* prev = box->prevLeafChild();
+        if (!prev || prev->bidiLevel() < bidiLevelOfRun)
+            return RenderedPosition(box->renderer(), box, box->caretLeftmostOffset());
+        box = prev;
+    } while (box);
+
+    ASSERT_NOT_REACHED();
+    return RenderedPosition();
+}
+
+RenderedPosition RenderedPosition::rightBoundaryOfBidiRun(unsigned char bidiLevelOfRun)
+{
+    if (!m_inlineBox || bidiLevelOfRun > m_inlineBox->bidiLevel())
+        return RenderedPosition();
+
+    InlineBox* box = m_inlineBox;
+    do {
+        InlineBox* next = box->nextLeafChild();
+        if (!next || next->bidiLevel() < bidiLevelOfRun)
+            return RenderedPosition(box->renderer(), box, box->caretRightmostOffset());
+        box = next;
+    } while (box);
+
+    ASSERT_NOT_REACHED();
+    return RenderedPosition();
+}
+
+bool RenderedPosition::atLeftBoundaryOfBidiRun(ShouldMatchBidiLevel shouldMatchBidiLevel, unsigned char bidiLevelOfRun) const
+{
+    if (!m_inlineBox)
+        return false;
+
+    if (atLeftmostOffsetInBox()) {
+        if (shouldMatchBidiLevel == IgnoreBidiLevel)
+            return !prevLeafChild() || prevLeafChild()->bidiLevel() < m_inlineBox->bidiLevel();
+        return m_inlineBox->bidiLevel() >= bidiLevelOfRun && (!prevLeafChild() || prevLeafChild()->bidiLevel() < bidiLevelOfRun);
+    }
+
+    if (atRightmostOffsetInBox()) {
+        if (shouldMatchBidiLevel == IgnoreBidiLevel)
+            return nextLeafChild() && m_inlineBox->bidiLevel() < nextLeafChild()->bidiLevel();
+        return nextLeafChild() && m_inlineBox->bidiLevel() < bidiLevelOfRun && nextLeafChild()->bidiLevel() >= bidiLevelOfRun;
+    }
+
+    return false;
+}
+
+bool RenderedPosition::atRightBoundaryOfBidiRun(ShouldMatchBidiLevel shouldMatchBidiLevel, unsigned char bidiLevelOfRun) const
+{
+    if (!m_inlineBox)
+        return false;
+
+    if (atRightmostOffsetInBox()) {
+        if (shouldMatchBidiLevel == IgnoreBidiLevel)
+            return !nextLeafChild() || nextLeafChild()->bidiLevel() < m_inlineBox->bidiLevel();
+        return m_inlineBox->bidiLevel() >= bidiLevelOfRun && (!nextLeafChild() || nextLeafChild()->bidiLevel() < bidiLevelOfRun);
+    }
+
+    if (atLeftmostOffsetInBox()) {
+        if (shouldMatchBidiLevel == IgnoreBidiLevel)
+            return prevLeafChild() && m_inlineBox->bidiLevel() < prevLeafChild()->bidiLevel();
+        return prevLeafChild() && m_inlineBox->bidiLevel() < bidiLevelOfRun && prevLeafChild()->bidiLevel() >= bidiLevelOfRun;
+    }
+
+    return false;
+}
+
+Position RenderedPosition::positionAtLeftBoundaryOfBiDiRun() const
+{
+    ASSERT(atLeftBoundaryOfBidiRun());
+
+    if (atLeftmostOffsetInBox())
+        return createLegacyEditingPosition(m_renderer->node(), m_offset);
+
+    return createLegacyEditingPosition(nextLeafChild()->renderer()->node(), m_offset);
+}
+
+Position RenderedPosition::positionAtRightBoundaryOfBiDiRun() const
+{
+    ASSERT(atRightBoundaryOfBidiRun());
+
+    if (atRightmostOffsetInBox())
+        return createLegacyEditingPosition(m_renderer->node(), m_offset);
+
+    return createLegacyEditingPosition(prevLeafChild()->renderer()->node(), m_offset);
+}
+
 LayoutRect RenderedPosition::absoluteRect(int* extraWidthToEndOfLine) const
 {
     if (isNull())
index d1eae62..6b84120 100644 (file)
@@ -46,25 +46,69 @@ public:
     RenderedPosition();
     explicit RenderedPosition(const VisiblePosition&);
     explicit RenderedPosition(const Position&, EAffinity);
+    bool isEquivalent(const RenderedPosition&) const;
 
     bool isNull() const { return !m_renderer; }
     RootInlineBox* rootBox() { return m_inlineBox ? m_inlineBox->root() : 0; }
 
+    unsigned char bidiLevelOnLeft() const;
+    unsigned char bidiLevelOnRight() const;
+    RenderedPosition leftBoundaryOfBidiRun(unsigned char bidiLevelOfRun);
+    RenderedPosition rightBoundaryOfBidiRun(unsigned char bidiLevelOfRun);
+
+    enum ShouldMatchBidiLevel { MatchBidiLevel, IgnoreBidiLevel };
+    bool atLeftBoundaryOfBidiRun() const { return atLeftBoundaryOfBidiRun(IgnoreBidiLevel, 0); }
+    bool atRightBoundaryOfBidiRun() const { return atRightBoundaryOfBidiRun(IgnoreBidiLevel, 0); }
+    // The following two functions return true only if the current position is at the end of the bidi run
+    // of the specified bidi embedding level.
+    bool atLeftBoundaryOfBidiRun(unsigned char bidiLevelOfRun) const { return atLeftBoundaryOfBidiRun(MatchBidiLevel, bidiLevelOfRun); }
+    bool atRightBoundaryOfBidiRun(unsigned char bidiLevelOfRun) const { return atRightBoundaryOfBidiRun(MatchBidiLevel, bidiLevelOfRun); }
+
+    Position positionAtLeftBoundaryOfBiDiRun() const;
+    Position positionAtRightBoundaryOfBiDiRun() const;
+
     LayoutRect absoluteRect() const { return absoluteRect(0); }
     LayoutRect absoluteRect(int& extraWidthToEndOfLine) const { return absoluteRect(&extraWidthToEndOfLine); }
 
 private:
+    bool operator==(const RenderedPosition&) const { return false; }
+    explicit RenderedPosition(RenderObject*, InlineBox*, int offset);
+
+    InlineBox* prevLeafChild() const;
+    InlineBox* nextLeafChild() const;
+    bool atLeftmostOffsetInBox() const { return m_inlineBox && m_offset == m_inlineBox->caretLeftmostOffset(); }
+    bool atRightmostOffsetInBox() const { return m_inlineBox && m_offset == m_inlineBox->caretRightmostOffset(); }
+    bool atLeftBoundaryOfBidiRun(ShouldMatchBidiLevel, unsigned char bidiLevelOfRun) const;
+    bool atRightBoundaryOfBidiRun(ShouldMatchBidiLevel, unsigned char bidiLevelOfRun) const;
+
     LayoutRect absoluteRect(int* extraWidthToEndOfLine) const;
 
     RenderObject* m_renderer;
     InlineBox* m_inlineBox;
     int m_offset;
+
+    static InlineBox* uncachedInlineBox() { return reinterpret_cast<InlineBox*>(1); }
+    // Needs to be different form 0 so pick 1 because it's also on the null page.
+
+    mutable InlineBox* m_prevLeafChild;
+    mutable InlineBox* m_nextLeafChild;
 };
 
 inline RenderedPosition::RenderedPosition()
     : m_renderer(0)
     , m_inlineBox(0)
     , m_offset(0)
+    , m_prevLeafChild(uncachedInlineBox())
+    , m_nextLeafChild(uncachedInlineBox())
+{
+}
+
+inline RenderedPosition::RenderedPosition(RenderObject* renderer, InlineBox* box, int offset)
+    : m_renderer(renderer)
+    , m_inlineBox(box)
+    , m_offset(offset)
+    , m_prevLeafChild(uncachedInlineBox())
+    , m_nextLeafChild(uncachedInlineBox())
 {
 }
 
index 100291c..8c810da 100644 (file)
@@ -69,6 +69,8 @@ public:
     
     VisiblePosition visibleStart() const { return VisiblePosition(m_start, isRange() ? DOWNSTREAM : affinity()); }
     VisiblePosition visibleEnd() const { return VisiblePosition(m_end, isRange() ? UPSTREAM : affinity()); }
+    VisiblePosition visibleBase() const { return VisiblePosition(m_base, isRange() ? (isBaseFirst() ? UPSTREAM : DOWNSTREAM) : affinity()); }
+    VisiblePosition visibleExtent() const { return VisiblePosition(m_extent, isRange() ? (isBaseFirst() ? DOWNSTREAM : UPSTREAM) : affinity()); }
 
     bool isNone() const { return selectionType() == NoSelection; }
     bool isCaret() const { return selectionType() == CaretSelection; }
index c59e6bf..429b920 100644 (file)
@@ -690,7 +690,8 @@ void EventHandler::updateSelectionForMouseDrag(const HitTestResult& hitTestResul
     if (m_frame->selection()->granularity() != CharacterGranularity)
         newSelection.expandUsingGranularity(m_frame->selection()->granularity());
 
-    m_frame->selection()->setNonDirectionalSelectionIfNeeded(newSelection, m_frame->selection()->granularity());
+    m_frame->selection()->setNonDirectionalSelectionIfNeeded(newSelection, m_frame->selection()->granularity(),
+        FrameSelection::AdjustEndpointsAtBidiBoundary);
 }
 #endif // ENABLE(DRAG_SUPPORT)