Skip to content
Permalink
Browse files
Upwards cursor movement incorrect when previous block ends with <br>
https://bugs.webkit.org/show_bug.cgi?id=33247

Reviewed by Tony Chang.

Source/WebCore:

The bug was caused by previousLinePosition's trying to obtain the root line box at the position
after the previous line's br. This obviously fails because the the position after br is considered
as a part of the next line.

Fixed the bug by obtaining root inline boxes using position at the minimum caret offset as supposed
to maximum caret offset. The code was initially introduced by r32508 to fix arrow key movement in RTL text
but the test added by the revision continues to pass after this change. Furthermore, this change makes
new code consistent with nextLinePosition.

Also reverted the change added by r55613 because it is no longer needed.

Tests: editing/execCommand/move-selection-back-line-rtl.html
       editing/execCommand/move-selection-back-line-strict.html

* editing/visible_units.cpp:
(WebCore::previousLinePosition):

LayoutTests:

Added tests to ensure WebKit lets user move caret up in strict mode and in RTL content
when the previous line ends with a br.

* editing/execCommand/move-selection-back-line-rtl-expected.txt: Copied from
LayoutTests/editing/execCommand/move-selection-back-line-expected.txt.
* editing/execCommand/move-selection-back-line-rtl.html: Copied from
LayoutTests/editing/execCommand/move-selection-back-line.html.
* editing/execCommand/move-selection-back-line-strict-expected.txt: Copied from
LayoutTests/editing/execCommand/move-selection-back-line-expected.txt.
* editing/execCommand/move-selection-back-line-strict.html: Copied from
LayoutTests/editing/execCommand/move-selection-back-line.html.


Canonical link: https://commits.webkit.org/81623@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@92526 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
rniwa committed Aug 5, 2011
1 parent 83d3a78 commit 3307caf16d176d83ce543d1014db16b802a0617c
Showing 7 changed files with 110 additions and 18 deletions.
@@ -1,3 +1,22 @@
2011-08-05 Ryosuke Niwa <rniwa@webkit.org>

Upwards cursor movement incorrect when previous block ends with <br>
https://bugs.webkit.org/show_bug.cgi?id=33247

Reviewed by Tony Chang.

Added tests to ensure WebKit lets user move caret up in strict mode and in RTL content
when the previous line ends with a br.

* editing/execCommand/move-selection-back-line-rtl-expected.txt: Copied from
LayoutTests/editing/execCommand/move-selection-back-line-expected.txt.
* editing/execCommand/move-selection-back-line-rtl.html: Copied from
LayoutTests/editing/execCommand/move-selection-back-line.html.
* editing/execCommand/move-selection-back-line-strict-expected.txt: Copied from
LayoutTests/editing/execCommand/move-selection-back-line-expected.txt.
* editing/execCommand/move-selection-back-line-strict.html: Copied from
LayoutTests/editing/execCommand/move-selection-back-line.html.

2011-08-05 Anders Carlsson <andersca@apple.com>

Add more Lion specific test results and add more tests to the skipped list.
@@ -0,0 +1,8 @@
Move the caret upwards from [ ] on the second line. The caret should appear in the green box inside [ ] on the first line.

קו [ ] ראשון

קו [ ] שני

PASS

@@ -0,0 +1,24 @@
<!DOCTYPE html>
<head>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
</head>
<body>
<p>Move the caret upwards from [ ] on the second line. The caret should appear in the green box inside [ ] on the first line.</p>
<div dir="rtl" contentEditable="true" style="font-family: monospace;">
<p dir="rtl">קו <span id="target" style="background-color:green">[ ]</span> ראשון<br></p>
<p dir="rtl">קו <span id="test">[ ]</span> שני<br></p>
</div>
<div id="results">FAILED</div>
<script src="../editing.js"></script>
<script>
function editingTest()
{
execMoveSelectionForwardByCharacterCommand();
execMoveSelectionBackwardByLineCommand();

// Verify that we ended up in "target".
if (window.getSelection().anchorNode.parentNode == document.getElementById("target"))
document.getElementById("results").innerText = "PASS";
}
runDumpAsTextEditingTest(false);
</script>
@@ -0,0 +1,6 @@
first line.. test test test test -[ ] test test test

second line. Put your cursor here [ ] and press the up arrow. The cursor should appear in the green box on the first line

PASS

@@ -0,0 +1,22 @@
<!DOCTYPE html>
<html>
<body>
<div contentEditable="true" style="font-family: monospace;">
<p>first line.. test test test test -<span id="target" style="background-color:green">[ ]</span> test test test<br/></p>
<p>second line. Put your cursor here [<span id="test"> </span>] and press the up arrow. The cursor should appear in the green box on the first line</p>
</div>
<div id="results">FAILED</div>
<script src="../editing.js"></script>
<script>
function editingTest()
{
execMoveSelectionBackwardByLineCommand();

// Verify that we ended up in "target".
if (window.getSelection().anchorNode.parentNode == document.getElementById("target"))
document.getElementById("results").innerText = "PASS";
}
runDumpAsTextEditingTest(false);
</script>
</body>
</html>
@@ -1,3 +1,27 @@
2011-08-05 Ryosuke Niwa <rniwa@webkit.org>

Upwards cursor movement incorrect when previous block ends with <br>
https://bugs.webkit.org/show_bug.cgi?id=33247

Reviewed by Tony Chang.

The bug was caused by previousLinePosition's trying to obtain the root line box at the position
after the previous line's br. This obviously fails because the the position after br is considered
as a part of the next line.

Fixed the bug by obtaining root inline boxes using position at the minimum caret offset as supposed
to maximum caret offset. The code was initially introduced by r32508 to fix arrow key movement in RTL text
but the test added by the revision continues to pass after this change. Furthermore, this change makes
new code consistent with nextLinePosition.

Also reverted the change added by r55613 because it is no longer needed.

Tests: editing/execCommand/move-selection-back-line-rtl.html
editing/execCommand/move-selection-back-line-strict.html

* editing/visible_units.cpp:
(WebCore::previousLinePosition):

2011-08-05 James Robinson <jamesr@chromium.org>

[chromium] Accelerated canvas breaks when moving canvases or resources between Pages
@@ -217,12 +217,6 @@ static VisiblePosition nextBoundary(const VisiblePosition& c, BoundarySearchFunc
return VisiblePosition(pos, VP_UPSTREAM_IF_POSSIBLE);
}

static bool canHaveCursor(RenderObject* o)
{
return (o->isText() && toRenderText(o)->linesBoundingBox().height())
|| (o->isBox() && toRenderBox(o)->borderBoundingBox().height());
}

// ---------

static unsigned startWordBoundary(const UChar* characters, unsigned length, unsigned offset, BoundarySearchContextAvailability mayHaveMoreContext, bool& needMoreContext)
@@ -541,19 +535,14 @@ VisiblePosition previousLinePosition(const VisiblePosition &visiblePosition, int
break;
Position pos = createLegacyEditingPosition(n, caretMinOffset(n));
if (pos.isCandidate()) {
RenderObject* o = n->renderer();
ASSERT(o);
if (canHaveCursor(o)) {
Position maxPos = createLegacyEditingPosition(n, caretMaxOffset(n));
maxPos.getInlineBoxAndOffset(DOWNSTREAM, box, ignoredCaretOffset);
if (box) {
// previous root line box found
root = box->root();
break;
}

return VisiblePosition(pos, DOWNSTREAM);
pos.getInlineBoxAndOffset(DOWNSTREAM, box, ignoredCaretOffset);
if (box) {
// previous root line box found
root = box->root();
break;
}

return VisiblePosition(pos, DOWNSTREAM);
}
n = previousLeafWithSameEditability(n);
}

0 comments on commit 3307caf

Please sign in to comment.