Caret not shown at the end of line in overtype mode
authorsvillar@igalia.com <svillar@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 Sep 2014 10:17:16 +0000 (10:17 +0000)
committersvillar@igalia.com <svillar@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 Sep 2014 10:17:16 +0000 (10:17 +0000)
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.

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

LayoutTests/ChangeLog
LayoutTests/editing/selection/block-cursor-overtype-mode-end-of-line-expected.html [new file with mode: 0644]
LayoutTests/editing/selection/block-cursor-overtype-mode-end-of-line-rtl-expected.html [new file with mode: 0644]
LayoutTests/editing/selection/block-cursor-overtype-mode-end-of-line-rtl.html [new file with mode: 0644]
LayoutTests/editing/selection/block-cursor-overtype-mode-end-of-line.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/FrameSelection.cpp
Source/WebCore/editing/VisibleUnits.cpp
Source/WebCore/editing/VisibleUnits.h

index 89f69f3..59ab1a6 100644 (file)
@@ -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
new file mode 100644 (file)
index 0000000..08841d0
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..8afe887
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..534097d
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..88f54e5
--- /dev/null
@@ -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>
index c07c3ea..ea48120 100644 (file)
@@ -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  Brian J. Burg  <burg@cs.washington.edu>
 
         Web Inspector: InjectedScripts should not be profiled or displayed in Timeline
index c17f6b9..f4b2168 100644 (file)
@@ -1806,17 +1806,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.
@@ -1842,7 +1838,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();
index 9100056..bdf1326 100644 (file)
@@ -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();
index 1755728..ed9655f 100644 (file)
@@ -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);