Added: trunk/LayoutTests/editing/selection/caret-at-bidi-boundary.html (0 => 93369)
--- trunk/LayoutTests/editing/selection/caret-at-bidi-boundary.html (rev 0)
+++ trunk/LayoutTests/editing/selection/caret-at-bidi-boundary.html 2011-08-18 23:07:15 UTC (rev 93369)
@@ -0,0 +1,108 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta http-equiv="content-type" content="text/html; charset=utf-8">
+<link rel="stylesheet" href=""
+<script src=""
+<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 place caret around bidirectional text properly.</p>
+<div>Current offset: <span id="log"></span></div>
+<dl id="tests">
+<dt contenteditable>abcאבג</dt>
+<dd>0 1 2 3 5 4 6</dd>
+
+<dt contenteditable>נת12</dt>
+<dd>2 3 4 1 4 (There is a bug. Clicking on the left of 12 should result in offset 0.)</dd>
+
+<dt contenteditable>לש<b>נ</b>ת</dt>
+<dd>0 3 2 1 4</dd>
+
+<dt contenteditable>aקל12יםd</dt>
+<dd>0 1 6 3 4 5 2 7 8</dd>
+
+<dt><span dir="rtl">12<b>קל43</b></span>ab</dt>
+<dd>4 5 6 3 0 1 2 7 8</dd>
+
+<dt dir="rtl"><span dir="ltr">abcלשנ</span></dt>
+<dd>6 1 2 6 5 4 3</dd>
+
+</dl>
+<div id="console"></div>
+<pre><script>
+
+// This function converts (node, offset) pair to a global offset (like TextIterator index).
+function globalOffsetFromNodeOffsetPair(node, offsetInNode) {
+ var tests = document.getElementById('tests');
+ var testContainer = node;
+ while (testContainer && testContainer.parentNode != tests)
+ testContainer = testContainer.parentNode;
+ if (!testContainer)
+ return 'a node outside of tests';
+
+ return testContainer.textContent.indexOf(node.textContent) + offsetInNode;
+}
+
+function runTest(target, expectations) {
+ var y = target.offsetTop + target.offsetHeight / 2;
+
+ var previousOffset = -1;
+ var j = 0;
+ for (var x = 5; x <= target.offsetWidth - 5; x++) {
+ eventSender.leapForward(1000);
+ eventSender.mouseMoveTo(target.offsetLeft + x, y);
+ eventSender.mouseDown();
+ eventSender.mouseUp();
+
+ var currentOffset = globalOffsetFromNodeOffsetPair(getSelection().baseNode, getSelection().baseOffset);
+ if (!getSelection().isCollapsed)
+ testFailed('selection was not collapsed');
+ else if (previousOffset == currentOffset)
+ continue;
+ else {
+ previousOffset = currentOffset;
+
+ if (expectations[j] != currentOffset)
+ testFailed('caret was at ' + currentOffset + ' but expected to be at ' + expectations[j]);
+ else {
+ testPassed('caret is at ' + currentOffset);
+ j++;
+ }
+ }
+ }
+
+ if (j < expectations.length)
+ testFailed('caret never reached offset' + expectations[j]);
+}
+
+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++) {
+ debug('Test "' + tests[i].textContent + '":');
+ runTest(tests[i], testExpectations[i].textContent.replace(/\s*\(.+\)\s*/, '').split(/\s+/).map(function (x) {return parseInt(x);}));
+ debug('');
+ }
+} else {
+ debug('This test requires eventSender');
+ document._onselectionchange_ = function () {
+ var selection = window.getSelection();
+ document.getElementById('log').textContent = globalOffsetFromNodeOffsetPair(selection.baseNode, selection.baseOffset) + ', ' +
+ globalOffsetFromNodeOffsetPair(selection.extentNode, selection.extentOffset);
+ }
+}
+
+var successfullyParsed = true;
+
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (93368 => 93369)
--- trunk/Source/WebCore/ChangeLog 2011-08-18 23:05:42 UTC (rev 93368)
+++ trunk/Source/WebCore/ChangeLog 2011-08-18 23:07:15 UTC (rev 93369)
@@ -1,3 +1,33 @@
+2011-08-18 Ryosuke Niwa <[email protected]>
+
+ positionForPoint returns wrong VisiblePosition at bidi boundaries
+ https://bugs.webkit.org/show_bug.cgi?id=65356
+
+ Reviewed by David Hyatt.
+
+ The bug was caused by RenderText::positionForPoint's assuming that the position will always reside
+ inside the inline box that contains the point, which is not true at the boundaries of bidi-runs.
+
+ For example, in aDC12BAb where AB12CD is a RTL text, the offset on the right of A is 7 even though
+ the inline box for "BA" only contains offsets 1, 2, and 3. We must traverse the bidi-run "DC12BA"
+ until the end to obtain the offset 7 from the inline box for "DC".
+
+ Fixed the bug by introducing createVisiblePositionAfterAdjustingOffsetForBiDi which traverses runs
+ on the left or the right of the position to compute the appropriate offset following the NSTextView convention.
+
+ This patch also fixes a regression from r74971 that caret is placed incorrectly between inline boxes of
+ LTR or RTL text in a RTL or LTR block respectively.
+
+ Test: editing/selection/caret-at-bidi-boundary.html
+
+ * rendering/InlineTextBox.cpp:
+ (WebCore::InlineTextBox::offsetForPosition):
+ * rendering/RenderText.cpp:
+ (WebCore::lineDirectionPointFitsInBox): Takes ShouldAffinityBeDownstream instead of EAfinity.
+ (WebCore::createVisiblePositionForBox):
+ (WebCore::createVisiblePositionAfterAdjustingOffsetForBiDi):
+ (WebCore::RenderText::positionForPoint):
+
2011-08-18 Xiaomei Ji <[email protected]>
--webkit-visual-word renaming right/leftWordPositionAcrossBoundary
Modified: trunk/Source/WebCore/rendering/InlineTextBox.cpp (93368 => 93369)
--- trunk/Source/WebCore/rendering/InlineTextBox.cpp 2011-08-18 23:05:42 UTC (rev 93368)
+++ trunk/Source/WebCore/rendering/InlineTextBox.cpp 2011-08-18 23:07:15 UTC (rev 93369)
@@ -1228,26 +1228,17 @@
if (isLineBreak())
return 0;
- int leftOffset = isLeftToRightDirection() ? 0 : m_len;
- int rightOffset = isLeftToRightDirection() ? m_len : 0;
- bool blockIsInOppositeDirection = renderer()->containingBlock()->style()->isLeftToRightDirection() != isLeftToRightDirection();
- if (blockIsInOppositeDirection)
- swap(leftOffset, rightOffset);
-
if (lineOffset - logicalLeft() > logicalWidth())
- return rightOffset;
+ return isLeftToRightDirection() ? len() : 0;
if (lineOffset - logicalLeft() < 0)
- return leftOffset;
+ return isLeftToRightDirection() ? 0 : len();
FontCachePurgePreventer fontCachePurgePreventer;
RenderText* text = toRenderText(renderer());
RenderStyle* style = text->style(m_firstLine);
const Font& font = style->font();
- int offset = font.offsetForPosition(constructTextRun(style, font), lineOffset - logicalLeft(), includePartialGlyphs);
- if (blockIsInOppositeDirection && (!offset || offset == m_len))
- return !offset ? m_len : 0;
- return offset;
+ return font.offsetForPosition(constructTextRun(style, font), lineOffset - logicalLeft(), includePartialGlyphs);
}
float InlineTextBox::positionForOffset(int offset) const
Modified: trunk/Source/WebCore/rendering/RenderText.cpp (93368 => 93369)
--- trunk/Source/WebCore/rendering/RenderText.cpp 2011-08-18 23:05:42 UTC (rev 93368)
+++ trunk/Source/WebCore/rendering/RenderText.cpp 2011-08-18 23:07:15 UTC (rev 93369)
@@ -431,9 +431,11 @@
return s;
}
-static bool lineDirectionPointFitsInBox(int pointLineDirection, InlineTextBox* box, int offset, EAffinity& affinity)
+enum ShouldAffinityBeDownstream { AlwaysDownstream, AlwaysUpstream, UpstreamIfPositionIsNotAtStart };
+
+static bool lineDirectionPointFitsInBox(int pointLineDirection, InlineTextBox* box, ShouldAffinityBeDownstream& shouldAffinityBeDownstream)
{
- affinity = DOWNSTREAM;
+ shouldAffinityBeDownstream = AlwaysDownstream;
// the x coordinate is equal to the left edge of this box
// the affinity must be downstream so the position doesn't jump back to the previous line
@@ -443,7 +445,7 @@
// and the x coordinate is to the left of the right edge of this box
// check to see if position goes in this box
if (pointLineDirection < box->logicalRight()) {
- affinity = offset > 0 ? VP_UPSTREAM_IF_POSSIBLE : DOWNSTREAM;
+ shouldAffinityBeDownstream = UpstreamIfPositionIsNotAtStart;
return true;
}
@@ -456,13 +458,104 @@
// box is last on line
// and the x coordinate is to the right of the last text box right edge
// generate VisiblePosition, use UPSTREAM affinity if possible
- affinity = offset > 0 ? VP_UPSTREAM_IF_POSSIBLE : DOWNSTREAM;
+ shouldAffinityBeDownstream = UpstreamIfPositionIsNotAtStart;
return true;
}
return false;
}
+static VisiblePosition createVisiblePositionForBox(const InlineBox* box, int offset, ShouldAffinityBeDownstream shouldAffinityBeDownstream)
+{
+ EAffinity affinity = VP_DEFAULT_AFFINITY;
+ switch (shouldAffinityBeDownstream) {
+ case AlwaysDownstream:
+ affinity = DOWNSTREAM;
+ break;
+ case AlwaysUpstream:
+ affinity = VP_UPSTREAM_IF_POSSIBLE;
+ break;
+ case UpstreamIfPositionIsNotAtStart:
+ affinity = offset > box->caretMinOffset() ? VP_UPSTREAM_IF_POSSIBLE : DOWNSTREAM;
+ break;
+ }
+ return box->renderer()->createVisiblePosition(offset, affinity);
+}
+
+static VisiblePosition createVisiblePositionAfterAdjustingOffsetForBiDi(const InlineTextBox* box, int offset, ShouldAffinityBeDownstream shouldAffinityBeDownstream)
+{
+ ASSERT(box);
+ ASSERT(box->renderer());
+ ASSERT(offset >= 0);
+
+ if (offset && static_cast<unsigned>(offset) < box->len())
+ return createVisiblePositionForBox(box, box->start() + offset, shouldAffinityBeDownstream);
+
+ bool positionIsAtStartOfBox = !offset;
+ if (positionIsAtStartOfBox == box->isLeftToRightDirection()) {
+ // offset is on the left edge
+
+ const InlineBox* prevBox = box->prevLeafChild();
+ if ((prevBox && prevBox->bidiLevel() == box->bidiLevel())
+ || box->renderer()->containingBlock()->style()->direction() == box->direction()) // FIXME: left on 12CBA
+ return createVisiblePositionForBox(box, box->caretLeftmostOffset(), shouldAffinityBeDownstream);
+
+ if (prevBox && prevBox->bidiLevel() > box->bidiLevel()) {
+ // e.g. left of B in aDC12BAb
+ const InlineBox* leftmostBox;
+ do {
+ leftmostBox = prevBox;
+ prevBox = leftmostBox->prevLeafChild();
+ } while (prevBox && prevBox->bidiLevel() > box->bidiLevel());
+ return createVisiblePositionForBox(leftmostBox, leftmostBox->caretRightmostOffset(), shouldAffinityBeDownstream);
+ }
+
+ if (!prevBox || prevBox->bidiLevel() < box->bidiLevel()) {
+ // e.g. left of D in aDC12BAb
+ const InlineBox* rightmostBox;
+ const InlineBox* nextBox = box;
+ do {
+ rightmostBox = nextBox;
+ nextBox = rightmostBox->nextLeafChild();
+ } while (nextBox && nextBox->bidiLevel() >= box->bidiLevel());
+ return createVisiblePositionForBox(rightmostBox,
+ box->isLeftToRightDirection() ? rightmostBox->caretMaxOffset() : rightmostBox->caretMinOffset(), shouldAffinityBeDownstream);
+ }
+
+ return createVisiblePositionForBox(box, box->caretRightmostOffset(), shouldAffinityBeDownstream);
+ }
+
+ const InlineBox* nextBox = box->nextLeafChild();
+ if ((nextBox && nextBox->bidiLevel() == box->bidiLevel())
+ || box->renderer()->containingBlock()->style()->direction() == box->direction())
+ return createVisiblePositionForBox(box, box->caretRightmostOffset(), shouldAffinityBeDownstream);
+
+ // offset is on the right edge
+ if (nextBox && nextBox->bidiLevel() > box->bidiLevel()) {
+ // e.g. right of C in aDC12BAb
+ const InlineBox* rightmostBox;
+ do {
+ rightmostBox = nextBox;
+ nextBox = rightmostBox->nextLeafChild();
+ } while (nextBox && nextBox->bidiLevel() > box->bidiLevel());
+ return createVisiblePositionForBox(rightmostBox, rightmostBox->caretLeftmostOffset(), shouldAffinityBeDownstream);
+ }
+
+ if (!nextBox || nextBox->bidiLevel() < box->bidiLevel()) {
+ // e.g. right of A in aDC12BAb
+ const InlineBox* leftmostBox;
+ const InlineBox* prevBox = box;
+ do {
+ leftmostBox = prevBox;
+ prevBox = leftmostBox->prevLeafChild();
+ } while (prevBox && prevBox->bidiLevel() >= box->bidiLevel());
+ return createVisiblePositionForBox(leftmostBox,
+ box->isLeftToRightDirection() ? leftmostBox->caretMinOffset() : leftmostBox->caretMaxOffset(), shouldAffinityBeDownstream);
+ }
+
+ return createVisiblePositionForBox(box, box->caretLeftmostOffset(), shouldAffinityBeDownstream);
+}
+
VisiblePosition RenderText::positionForPoint(const LayoutPoint& point)
{
if (!firstTextBox() || textLength() == 0)
@@ -479,13 +572,13 @@
// at the y coordinate of the first line or above
// and the x coordinate is to the left of the first text box left edge
offset = firstTextBox()->offsetForPosition(pointLineDirection);
- return createVisiblePosition(offset + firstTextBox()->start(), offset > 0 ? VP_UPSTREAM_IF_POSSIBLE : DOWNSTREAM);
+ return createVisiblePositionAfterAdjustingOffsetForBiDi(firstTextBox(), offset, UpstreamIfPositionIsNotAtStart);
}
if (lastTextBox() && pointBlockDirection >= lastTextBox()->root()->selectionTop() && pointLineDirection >= lastTextBox()->logicalRight()) {
// at the y coordinate of the last line or below
// and the x coordinate is to the right of the last text box right edge
offset = lastTextBox()->offsetForPosition(pointLineDirection);
- return createVisiblePosition(offset + lastTextBox()->start(), VP_UPSTREAM_IF_POSSIBLE);
+ return createVisiblePositionAfterAdjustingOffsetForBiDi(lastTextBox(), offset, AlwaysUpstream);
}
InlineTextBox* lastBoxAbove = 0;
@@ -497,10 +590,10 @@
bottom = min(bottom, rootBox->nextRootBox()->lineTop());
if (pointBlockDirection < bottom) {
- EAffinity affinity;
- int offset = box->offsetForPosition(pointLineDirection);
- if (lineDirectionPointFitsInBox(pointLineDirection, box, offset, affinity))
- return createVisiblePosition(offset + box->start(), affinity);
+ ShouldAffinityBeDownstream shouldAffinityBeDownstream;
+ offset = box->offsetForPosition(pointLineDirection);
+ if (lineDirectionPointFitsInBox(pointLineDirection, box, shouldAffinityBeDownstream))
+ return createVisiblePositionAfterAdjustingOffsetForBiDi(box, offset, shouldAffinityBeDownstream);
}
lastBoxAbove = box;
}
