Merge r174097 - Caret not shown at the end of line in overtype mode

https://bugs.webkit.org/show_bug.cgi?id=135508 Reviewed by Ryosuke Niwa. Source/WebCore: When overtype mode is enabled we usually replace the 'normal' blinking caret shown in contenteditable elements by a block cursor that covers the next character to be replaced. The exception is the end of line where we fallback to the blinking caret even in overtype mode (as there is no next character to replace). We were not showing anything at the end of lines in overtype mode because the detection of the end of line was incorrect and not very understandable. Replaced the old code with a proper and clean end of line detection mechanism compatible with bidi text. Tests: editing/selection/block-cursor-overtype-mode-end-of-line-rtl.html editing/selection/block-cursor-overtype-mode-end-of-line.html * editing/FrameSelection.cpp: (WebCore::FrameSelection::updateAppearance): * editing/VisibleUnits.cpp: (WebCore::isLogicalEndOfLine): * editing/VisibleUnits.h: LayoutTests: * editing/selection/block-cursor-overtype-mode-end-of-line-expected.html: Added. * editing/selection/block-cursor-overtype-mode-end-of-line-rtl-expected.html: Added. * editing/selection/block-cursor-overtype-mode-end-of-line-rtl.html: Added. * editing/selection/block-cursor-overtype-mode-end-of-line.html: Added. Canonical link: https://commits.webkit.org/154760.61@webkitgtk/2.6 git-svn-id: https://svn.webkit.org/repository/webkit/releases/WebKitGTK/webkit-2.6@174449 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Oct 8, 2014 · 864f0f4 · 864f0f4
1 parent 35e9560
commit 864f0f4
Show file tree

Hide file tree

Showing 9 changed files with 119 additions and 7 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2014-08-01  Sergio Villar Senin  <svillar@igalia.com>
+
+        Caret not shown at the end of line in overtype mode
+        https://bugs.webkit.org/show_bug.cgi?id=135508
+
+        Reviewed by Ryosuke Niwa.
+
+        * editing/selection/block-cursor-overtype-mode-end-of-line-expected.html: Added.
+        * editing/selection/block-cursor-overtype-mode-end-of-line-rtl-expected.html: Added.
+        * editing/selection/block-cursor-overtype-mode-end-of-line-rtl.html: Added.
+        * editing/selection/block-cursor-overtype-mode-end-of-line.html: Added.
+
 2014-09-29  David Hyatt  <hyatt@apple.com>
 
         REGRESSION (r168046): Confused column spans when combined with dynamic animations

diff --git a/LayoutTests/editing/selection/block-cursor-overtype-mode-end-of-line-expected.html b/LayoutTests/editing/selection/block-cursor-overtype-mode-end-of-line-expected.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+function run() {
+    var element = (document.getElementById("editableContent")).firstChild;
+    getSelection().collapse(element, element.length);
+}
+</script>
+</head>
+<body style = "font: 30px;" onload="run();">
+    <!-- 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>
+</html>
diff --git a/LayoutTests/editing/selection/block-cursor-overtype-mode-end-of-line-rtl-expected.html b/LayoutTests/editing/selection/block-cursor-overtype-mode-end-of-line-rtl-expected.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+function run() {
+    var element = (document.getElementById("editableContent")).firstChild;
+    getSelection().collapse(element, element.length);
+}
+</script>
+</head>
+<body style = "font: 30px;" onload="run();">
+    <!-- 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>
+</html>
diff --git a/LayoutTests/editing/selection/block-cursor-overtype-mode-end-of-line-rtl.html b/LayoutTests/editing/selection/block-cursor-overtype-mode-end-of-line-rtl.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner)
+    internals.toggleOverwriteModeEnabled(document);
+
+function run() {
+    var element = (document.getElementById("editableContent")).firstChild;
+    getSelection().collapse(element, element.length);
+}
+</script>
+</head>
+<body style = "font: 30px;" onload="run();"">
+    <!-- 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>
+</html>
diff --git a/LayoutTests/editing/selection/block-cursor-overtype-mode-end-of-line.html b/LayoutTests/editing/selection/block-cursor-overtype-mode-end-of-line.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner)
+    internals.toggleOverwriteModeEnabled(document);
+
+function run() {
+    var element = (document.getElementById("editableContent")).firstChild;
+    getSelection().collapse(element, element.length);
+}
+</script>
+</head>
+<body style = "font: 30px;" onload="run();"">
+    <!-- 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>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,30 @@
+2014-08-01  Sergio Villar Senin  <svillar@igalia.com>
+
+        Caret not shown at the end of line in overtype mode
+        https://bugs.webkit.org/show_bug.cgi?id=135508
+
+        Reviewed by Ryosuke Niwa.
+
+        When overtype mode is enabled we usually replace the 'normal'
+        blinking caret shown in contenteditable elements by a block cursor
+        that covers the next character to be replaced. The exception is the
+        end of line where we fallback to the blinking caret even in overtype
+        mode (as there is no next character to replace).
+
+        We were not showing anything at the end of lines in overtype mode
+        because the detection of the end of line was incorrect and not very
+        understandable. Replaced the old code with a proper and clean end of
+        line detection mechanism compatible with bidi text.
+
+        Tests: editing/selection/block-cursor-overtype-mode-end-of-line-rtl.html
+               editing/selection/block-cursor-overtype-mode-end-of-line.html
+
+        * editing/FrameSelection.cpp:
+        (WebCore::FrameSelection::updateAppearance):
+        * editing/VisibleUnits.cpp:
+        (WebCore::isLogicalEndOfLine):
+        * editing/VisibleUnits.h:
+
 2014-09-29  David Hyatt  <hyatt@apple.com>
 
         REGRESSION (r168046): Confused column spans when combined with dynamic animations

diff --git a/Source/WebCore/editing/FrameSelection.cpp b/Source/WebCore/editing/FrameSelection.cpp
@@ -1808,17 +1808,13 @@ 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
     // the FrameSelection will paint a blinking caret as usual).
     VisibleSelection oldSelection = selection();
-    VisiblePosition forwardPosition;
-    if (m_shouldShowBlockCursor && oldSelection.isCaret()) {
-        forwardPosition = modifyExtendingForward(CharacterGranularity);
-        m_caretPaint = forwardPosition.isNull();
-    }
 
 #if ENABLE(TEXT_CARET)
+    bool paintBlockCursor = m_shouldShowBlockCursor && m_selection.isCaret() && !isLogicalEndOfLine(m_selection.visibleEnd());
     bool caretRectChangedOrCleared = recomputeCaretRect();
 
     bool caretBrowsing = m_frame->settings().caretBrowsingEnabled();
-    bool shouldBlink = caretIsVisible() && isCaret() && (oldSelection.isContentEditable() || caretBrowsing) && forwardPosition.isNull();
+    bool shouldBlink = !paintBlockCursor && caretIsVisible() && isCaret() && (oldSelection.isContentEditable() || caretBrowsing);
 
     // If the caret moved, stop the blink timer so we can restart with a
     // black caret in the new location.
@@ -1844,7 +1840,12 @@ void FrameSelection::updateAppearance()
 
     // 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>.
-    VisibleSelection selection(oldSelection.visibleStart(), forwardPosition.isNotNull() ? forwardPosition : oldSelection.visibleEnd());
+#if ENABLE(TEXT_CARET)
+    VisiblePosition endVisiblePosition = paintBlockCursor ? modifyExtendingForward(CharacterGranularity) : oldSelection.visibleEnd();
+    VisibleSelection selection(oldSelection.visibleStart(), endVisiblePosition);
+#else
+    VisibleSelection selection(oldSelection.visibleStart(), oldSelection.visibleEnd());
+#endif
 
     if (!selection.isRange()) {
         view->clearSelection();

diff --git a/Source/WebCore/editing/VisibleUnits.cpp b/Source/WebCore/editing/VisibleUnits.cpp
@@ -919,6 +919,11 @@ bool isEndOfLine(const VisiblePosition& 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)
 {
     RenderBlockFlow& containingBlock = root.blockFlow();

diff --git a/Source/WebCore/editing/VisibleUnits.h b/Source/WebCore/editing/VisibleUnits.h
@@ -62,6 +62,7 @@ WEBCORE_EXPORT bool isStartOfLine(const VisiblePosition &);
 WEBCORE_EXPORT bool isEndOfLine(const VisiblePosition &);
 VisiblePosition logicalStartOfLine(const VisiblePosition &);
 VisiblePosition logicalEndOfLine(const VisiblePosition &);
+bool isLogicalEndOfLine(const VisiblePosition &);
 VisiblePosition leftBoundaryOfLine(const VisiblePosition&, TextDirection);
 VisiblePosition rightBoundaryOfLine(const VisiblePosition&, TextDirection);