Text insertion cursor disappears after pressing enter
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 7 Apr 2017 15:05:43 +0000 (15:05 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 7 Apr 2017 15:05:43 +0000 (15:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=169291
<rdar://problem/30899611>

Reviewed by Tim Horton.

Source/WebCore:

Positon upstream/downstream (as the result of VisiblePosition -> canonicalPosition) require
linebox tree. In addition to regular text, we need to bail out of simple line layout on line breaks too.

Test: editing/simple-line-layout-caret-is-gone.html

* dom/Position.cpp:
(WebCore::ensureLineBoxesIfNeeded):
(WebCore::Position::upstream):
(WebCore::Position::downstream):
(WebCore::Position::getInlineBoxAndOffset):
* rendering/RenderLineBreak.cpp:
(WebCore::RenderLineBreak::ensureLineBoxes):
(WebCore::RenderLineBreak::positionForPoint):
(WebCore::RenderLineBreak::setSelectionState):
(WebCore::RenderLineBreak::collectSelectionRects):
(WebCore::ensureLineBoxes): Deleted.
* rendering/RenderLineBreak.h:

LayoutTests:

* editing/simple-line-layout-caret-is-gone-expected.txt: Added.
* editing/simple-line-layout-caret-is-gone.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/simple-line-layout-caret-is-gone-expected.txt [new file with mode: 0644]
LayoutTests/editing/simple-line-layout-caret-is-gone.html [new file with mode: 0644]
LayoutTests/platform/ios/editing/simple-line-layout-caret-is-gone-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/editing/pasteboard/5761530-1-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/dom/Position.cpp
Source/WebCore/rendering/RenderLineBreak.cpp
Source/WebCore/rendering/RenderLineBreak.h

index 78e21a9..76581ee 100644 (file)
@@ -1,3 +1,14 @@
+2017-04-07  Zalan Bujtas  <zalan@apple.com>
+
+        Text insertion cursor disappears after pressing enter
+        https://bugs.webkit.org/show_bug.cgi?id=169291
+        <rdar://problem/30899611>
+
+        Reviewed by Tim Horton.
+
+        * editing/simple-line-layout-caret-is-gone-expected.txt: Added.
+        * editing/simple-line-layout-caret-is-gone.html: Added.
+
 2017-04-06  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Make FontWithFeatures test font pass OTS
diff --git a/LayoutTests/editing/simple-line-layout-caret-is-gone-expected.txt b/LayoutTests/editing/simple-line-layout-caret-is-gone-expected.txt
new file mode 100644 (file)
index 0000000..ca48f3d
--- /dev/null
@@ -0,0 +1 @@
+36 0 1 18
diff --git a/LayoutTests/editing/simple-line-layout-caret-is-gone.html b/LayoutTests/editing/simple-line-layout-caret-is-gone.html
new file mode 100644 (file)
index 0000000..783e945
--- /dev/null
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that contenteditable returns the correct caret bounds.</title>
+<style>
+body {
+    margin: 0px;
+}
+
+#editable {
+    -webkit-nbsp-mode: normal !important;
+    -webkit-line-break: auto !important;
+    width: 50px;
+    height: 50px;
+}
+</style>
+</head>
+<body>
+<div id=editable contenteditable=true></div>
+<script>
+editable.focus();
+if (window.testRunner)
+    testRunner.dumpAsText();
+if (window.eventSender) {
+    eventSender.keyDown('\n');
+    eventSender.keyDown('\n');
+}
+if (window.internals) {
+       var withTextCaretRect = internals.absoluteCaretBounds();
+    document.body.innerText = withTextCaretRect.top + " " + withTextCaretRect.left + " " + withTextCaretRect.width + " " + withTextCaretRect.height;
+}
+</script>
+</body>
+</html>
diff --git a/LayoutTests/platform/ios/editing/simple-line-layout-caret-is-gone-expected.txt b/LayoutTests/platform/ios/editing/simple-line-layout-caret-is-gone-expected.txt
new file mode 100644 (file)
index 0000000..13620c0
--- /dev/null
@@ -0,0 +1 @@
+0 0 2 20
index e61026e..90085db 100644 (file)
@@ -1,3 +1,3 @@
 This tests to see that tabs are put into tab spans when they are copied individually. The pasted tab should be inside of a tab span, not a style span. To run the test manually, paste and then inspect the editable region, and ensure that there is a tab span at the beginning of the editable div.
 
-<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space:pre;"> </span>xxx
+<span class="Apple-tab-span" style="white-space: pre;">        </span><span class="Apple-tab-span" style="white-space:pre;">   </span>xxx
index e0d43b1..13fffc3 100644 (file)
@@ -1,3 +1,29 @@
+2017-04-07  Zalan Bujtas  <zalan@apple.com>
+
+        Text insertion cursor disappears after pressing enter
+        https://bugs.webkit.org/show_bug.cgi?id=169291
+        <rdar://problem/30899611>
+
+        Reviewed by Tim Horton.
+
+        Positon upstream/downstream (as the result of VisiblePosition -> canonicalPosition) require
+        linebox tree. In addition to regular text, we need to bail out of simple line layout on line breaks too.
+
+        Test: editing/simple-line-layout-caret-is-gone.html
+
+        * dom/Position.cpp:
+        (WebCore::ensureLineBoxesIfNeeded):
+        (WebCore::Position::upstream):
+        (WebCore::Position::downstream):
+        (WebCore::Position::getInlineBoxAndOffset):
+        * rendering/RenderLineBreak.cpp:
+        (WebCore::RenderLineBreak::ensureLineBoxes):
+        (WebCore::RenderLineBreak::positionForPoint):
+        (WebCore::RenderLineBreak::setSelectionState):
+        (WebCore::RenderLineBreak::collectSelectionRects):
+        (WebCore::ensureLineBoxes): Deleted.
+        * rendering/RenderLineBreak.h:
+
 2017-04-07  Xan Lopez  <xlopez@igalia.com>
 
         [GTK] Fix codec name in OWR ASSERT
index e0c6338..b6f5575 100644 (file)
@@ -634,6 +634,13 @@ static bool isStreamer(const PositionIterator& pos)
     return pos.atStartOfNode();
 }
 
+static void ensureLineBoxesIfNeeded(RenderObject& renderer)
+{
+    if (!is<RenderText>(renderer) && !is<RenderLineBreak>(renderer))
+        return;
+    is<RenderText>(renderer) ? downcast<RenderText>(renderer).ensureLineBoxes() : downcast<RenderLineBreak>(renderer).ensureLineBoxes();
+}
+
 // This function and downstream() are used for moving back and forth between visually equivalent candidates.
 // For example, for the text node "foo     bar" where whitespace is collapsible, there are two candidates 
 // that map to the VisiblePosition between 'b' and the space.  This function will return the left candidate 
@@ -679,7 +686,7 @@ Position Position::upstream(EditingBoundaryCrossingRule rule) const
         RenderObject* renderer = currentNode.renderer();
         if (!renderer || renderer->style().visibility() != VISIBLE)
             continue;
-                 
+        ensureLineBoxesIfNeeded(*renderer);
         if (rule == CanCrossEditingBoundary && boundaryCrossed) {
             lastVisible = currentPosition;
             break;
@@ -704,8 +711,6 @@ Position Position::upstream(EditingBoundaryCrossingRule rule) const
         // return current position if it is in rendered text
         if (is<RenderText>(*renderer)) {
             auto& textRenderer = downcast<RenderText>(*renderer);
-            textRenderer.ensureLineBoxes();
-
             if (!textRenderer.firstTextBox())
                 continue;
             if (&currentNode != startNode) {
@@ -816,7 +821,7 @@ Position Position::downstream(EditingBoundaryCrossingRule rule) const
         auto* renderer = currentNode.renderer();
         if (!renderer || renderer->style().visibility() != VISIBLE)
             continue;
-            
+        ensureLineBoxesIfNeeded(*renderer);
         if (rule == CanCrossEditingBoundary && boundaryCrossed) {
             lastVisible = currentPosition;
             break;
@@ -836,8 +841,6 @@ Position Position::downstream(EditingBoundaryCrossingRule rule) const
         // return current position if it is in rendered text
         if (is<RenderText>(*renderer)) {
             auto& textRenderer = downcast<RenderText>(*renderer);
-            textRenderer.ensureLineBoxes();
-
             if (!textRenderer.firstTextBox())
                 continue;
             if (&currentNode != startNode) {
@@ -1230,9 +1233,11 @@ void Position::getInlineBoxAndOffset(EAffinity affinity, TextDirection primaryDi
     caretOffset = deprecatedEditingOffset();
     RenderObject* renderer = deprecatedNode()->renderer();
 
-    if (renderer->isBR())
-        inlineBox = !caretOffset ? downcast<RenderLineBreak>(*renderer).inlineBoxWrapper() : nullptr;
-    else if (is<RenderText>(*renderer)) {
+    if (renderer->isBR()) {
+        auto& lineBreakRenderer = downcast<RenderLineBreak>(*renderer);
+        lineBreakRenderer.ensureLineBoxes();
+        inlineBox = !caretOffset ? lineBreakRenderer.inlineBoxWrapper() : nullptr;
+    } else if (is<RenderText>(*renderer)) {
         auto& textRenderer = downcast<RenderText>(*renderer);
         textRenderer.ensureLineBoxes();
 
index b819980..dced326 100644 (file)
@@ -48,13 +48,6 @@ static const SimpleLineLayout::Layout* simpleLineLayout(const RenderLineBreak& r
     return downcast<RenderBlockFlow>(*renderer.parent()).simpleLineLayout();
 }
 
-static void ensureLineBoxes(const RenderLineBreak& renderer)
-{
-    if (!is<RenderBlockFlow>(*renderer.parent()))
-        return;
-    downcast<RenderBlockFlow>(*renderer.parent()).ensureLineBoxes();
-}
-
 RenderLineBreak::RenderLineBreak(HTMLElement& element, RenderStyle&& style)
     : RenderBoxModelObject(element, WTFMove(style), 0)
     , m_inlineBoxWrapper(nullptr)
@@ -129,6 +122,13 @@ void RenderLineBreak::dirtyLineBoxes(bool fullLayout)
     m_inlineBoxWrapper->dirtyLineBoxes();
 }
 
+void RenderLineBreak::ensureLineBoxes()
+{
+    if (!is<RenderBlockFlow>(*parent()))
+        return;
+    downcast<RenderBlockFlow>(*parent()).ensureLineBoxes();
+}
+
 void RenderLineBreak::deleteLineBoxesBeforeSimpleLineLayout()
 {
     delete m_inlineBoxWrapper;
@@ -152,14 +152,14 @@ bool RenderLineBreak::canBeSelectionLeaf() const
 
 VisiblePosition RenderLineBreak::positionForPoint(const LayoutPoint&, const RenderRegion*)
 {
-    ensureLineBoxes(*this);
+    ensureLineBoxes();
     return createVisiblePosition(0, DOWNSTREAM);
 }
 
 void RenderLineBreak::setSelectionState(SelectionState state)
 {
     if (state != SelectionNone)
-        ensureLineBoxes(*this);
+        ensureLineBoxes();
     RenderBoxModelObject::setSelectionState(state);
     if (!m_inlineBoxWrapper)
         return;
@@ -228,7 +228,7 @@ void RenderLineBreak::updateFromStyle()
 #if PLATFORM(IOS)
 void RenderLineBreak::collectSelectionRects(Vector<SelectionRect>& rects, unsigned, unsigned)
 {
-    ensureLineBoxes(*this);
+    ensureLineBoxes();
     InlineElementBox* box = m_inlineBoxWrapper;
     if (!box)
         return;
index 26883a8..dd79c41 100644 (file)
@@ -54,6 +54,7 @@ public:
 #if PLATFORM(IOS)
 void collectSelectionRects(Vector<SelectionRect>&, unsigned startOffset = 0, unsigned endOffset = std::numeric_limits<unsigned>::max()) override;
 #endif
+    void ensureLineBoxes();
 
 private:
     void node() const = delete;