Commit 0b17d72f authored by svillar@igalia.com's avatar svillar@igalia.com

Invalid caret at the end of line in overtype mode

When using overtype mode in content editable content we usually replace the
caret by a block cursor that covers the next character to be replaced.
However at the end of lines we should show the 'normal' blinking caret. In
those cases we were incorrectly adding a selection block till the end of the
container of the editable content.

That's because we were using a complicated (and wrong) way to detect the end
of lines based on modifying the internal selection used by the overtype
mode. We now use the proper tools to detect the end of line detection which
also supports bidi text.

BUG=385003

Review URL: https://codereview.chromium.org/437843002

git-svn-id: svn://svn.chromium.org/blink/trunk@180002 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 8f30384b
<!DOCTYPE html>
<script>
function runTest() {
var element = (document.getElementById("editableContent")).firstChild;
getSelection().collapse(element, element.length);
}
</script>
<body onload="runTest();">
<!-- We want to test block cursor appearance at the end of lines that aren't the end of the editable element. -->
<div id="editableContent" contenteditable="true">ABC<br>ABC</div>
</body>
<!DOCTYPE html>
<script>
function runTest() {
var element = (document.getElementById("editableContent")).firstChild;
getSelection().collapse(element, element.length);
}
</script>
<body onload="runTest();">
<!-- We want to test block cursor appearance at the end of lines that aren't the end of the editable element. -->
<div id="editableContent" style="direction: rtl;" contenteditable="true">&#x05e9;&#x05d3;&#x05df;ABC<br>ABC</div>
</body>
<!DOCTYPE html>
<script>
if (window.testRunner)
internals.toggleOverwriteModeEnabled(document);
function runTest() {
var element = (document.getElementById("editableContent")).firstChild;
getSelection().collapse(element, element.length);
}
</script>
<body onload="runTest();">
<!-- We want to test block cursor appearance at the end of lines that aren't the end of the editable element. -->
<div id="editableContent" style="direction: rtl;" contenteditable="true">&#x05e9;&#x05d3;&#x05df;ABC<br>ABC</div>
</body>
<!DOCTYPE html>
<script>
if (window.testRunner)
internals.toggleOverwriteModeEnabled(document);
function runTest() {
var element = (document.getElementById("editableContent")).firstChild;
getSelection().collapse(element, element.length);
}
</script>
<body onload="runTest();">
<!-- We want to test block cursor appearance at the end of lines that aren't the end of the editable element. -->
<div id="editableContent" contenteditable="true">ABC<br>ABC</div>
</body>
...@@ -1544,14 +1544,10 @@ void FrameSelection::updateAppearance() ...@@ -1544,14 +1544,10 @@ void FrameSelection::updateAppearance()
{ {
// Paint a block cursor instead of a caret in overtype mode unless the caret is at the end of a line (in this case // Paint a block cursor instead of a caret in overtype mode unless the caret is at the end of a line (in this case
// the FrameSelection will paint a blinking caret as usual). // the FrameSelection will paint a blinking caret as usual).
VisiblePosition forwardPosition; bool paintBlockCursor = m_shouldShowBlockCursor && m_selection.isCaret() && !isLogicalEndOfLine(m_selection.visibleEnd());
if (m_shouldShowBlockCursor && m_selection.isCaret()) {
forwardPosition = modifyExtendingForward(CharacterGranularity);
m_caretPaint = forwardPosition.isNull();
}
bool caretRectChangedOrCleared = recomputeCaretRect(); bool caretRectChangedOrCleared = recomputeCaretRect();
bool shouldBlink = shouldBlinkCaret() && forwardPosition.isNull(); bool shouldBlink = !paintBlockCursor && shouldBlinkCaret();
// If the caret moved, stop the blink timer so we can restart with a // If the caret moved, stop the blink timer so we can restart with a
// black caret in the new location. // black caret in the new location.
...@@ -1581,7 +1577,8 @@ void FrameSelection::updateAppearance() ...@@ -1581,7 +1577,8 @@ void FrameSelection::updateAppearance()
// Construct a new VisibleSolution, since m_selection is not necessarily valid, and the following steps // Construct a new VisibleSolution, since m_selection is not necessarily valid, and the following steps
// assume a valid selection. See <https://bugs.webkit.org/show_bug.cgi?id=69563> and <rdar://problem/10232866>. // assume a valid selection. See <https://bugs.webkit.org/show_bug.cgi?id=69563> and <rdar://problem/10232866>.
VisibleSelection selection(m_selection.visibleStart(), forwardPosition.isNotNull() ? forwardPosition : m_selection.visibleEnd()); VisiblePosition endVisiblePosition = paintBlockCursor ? modifyExtendingForward(CharacterGranularity) : m_selection.visibleEnd();
VisibleSelection selection(m_selection.visibleStart(), endVisiblePosition);
if (!selection.isRange()) { if (!selection.isRange()) {
view->clearSelection(); view->clearSelection();
......
...@@ -896,6 +896,11 @@ bool isEndOfLine(const VisiblePosition &p) ...@@ -896,6 +896,11 @@ bool isEndOfLine(const VisiblePosition &p)
return p.isNotNull() && p == endOfLine(p); return p.isNotNull() && p == endOfLine(p);
} }
bool isLogicalEndOfLine(const VisiblePosition &p)
{
return p.isNotNull() && p == logicalEndOfLine(p);
}
static inline IntPoint absoluteLineDirectionPointToLocalPointInBlock(RootInlineBox* root, int lineDirectionPoint) static inline IntPoint absoluteLineDirectionPointToLocalPointInBlock(RootInlineBox* root, int lineDirectionPoint)
{ {
ASSERT(root); ASSERT(root);
......
...@@ -63,6 +63,7 @@ bool isStartOfLine(const VisiblePosition &); ...@@ -63,6 +63,7 @@ bool isStartOfLine(const VisiblePosition &);
bool isEndOfLine(const VisiblePosition &); bool isEndOfLine(const VisiblePosition &);
VisiblePosition logicalStartOfLine(const VisiblePosition &); VisiblePosition logicalStartOfLine(const VisiblePosition &);
VisiblePosition logicalEndOfLine(const VisiblePosition &); VisiblePosition logicalEndOfLine(const VisiblePosition &);
bool isLogicalEndOfLine(const VisiblePosition &);
VisiblePosition leftBoundaryOfLine(const VisiblePosition&, TextDirection); VisiblePosition leftBoundaryOfLine(const VisiblePosition&, TextDirection);
VisiblePosition rightBoundaryOfLine(const VisiblePosition&, TextDirection); VisiblePosition rightBoundaryOfLine(const VisiblePosition&, TextDirection);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment