REGRESSION(r164133): Selection disappears after scrolling on nytimes.com
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Jul 2014 01:45:54 +0000 (01:45 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Jul 2014 01:45:54 +0000 (01:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=135361

Reviewed by Ryosuke Niwa.

Ensure that when a RenderElement, part of the current selection is removed,
we recalculate and update the selection soon after layout.

Source/WebCore:
Test: fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed.html

* editing/FrameSelection.cpp:
(WebCore::FrameSelection::setNeedsSelectionUpdate):
(WebCore::FrameSelection::didLayout): didLayout name reflects its functionality better.
(WebCore::FrameSelection::layoutDidChange): Deleted.
* editing/FrameSelection.h: : move some functions to private.
* page/FrameView.cpp:
(WebCore::FrameView::performPostLayoutTasks):
* rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::willBeDestroyed):
* rendering/RenderElement.cpp:
(WebCore::RenderElement::removeChildInternal):
* rendering/RenderInline.cpp:
(WebCore::RenderInline::willBeDestroyed):

LayoutTests:
* fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed-expected.html: Added.
* fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed-expected.html [new file with mode: 0644]
LayoutTests/fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/FrameSelection.cpp
Source/WebCore/editing/FrameSelection.h
Source/WebCore/page/FrameView.cpp
Source/WebCore/rendering/RenderBlockFlow.cpp
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderInline.cpp

index f585e86..bbed778 100644 (file)
@@ -1,3 +1,16 @@
+2014-07-28  Zalan Bujtas  <zalan@apple.com>
+
+        REGRESSION(r164133): Selection disappears after scrolling on nytimes.com
+        https://bugs.webkit.org/show_bug.cgi?id=135361
+
+        Reviewed by Ryosuke Niwa.
+
+        Ensure that when a RenderElement, part of the current selection is removed,
+        we recalculate and update the selection soon after layout.
+
+        * fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed-expected.html: Added.
+        * fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed.html: Added.
+
 2014-07-28  Andreas Kling  <akling@apple.com>
 
         REGRESSION (r160806): CSS zoom property doesn't work on anything inside anchors.
diff --git a/LayoutTests/fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed-expected.html b/LayoutTests/fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed-expected.html
new file mode 100644 (file)
index 0000000..819fd7c
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<head>
+    <title>This tests that selection stays on when part of the selection gets removed.</title>
+</head>
+<body>
+    This should still be selected.
+    <script>
+        var range = document.createRange(); 
+        range.selectNode(document.body); 
+        window.getSelection().addRange(range);
+    </script>
+</body>
+</html>
diff --git a/LayoutTests/fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed.html b/LayoutTests/fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed.html
new file mode 100644 (file)
index 0000000..a3f8353
--- /dev/null
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<head>
+    <title>This tests that selection stays on when part of the selection gets removed.</title>
+</head>
+<body>
+    <div id=foo>When this text disappears, selection clears too.</div>
+    This should still be selected.
+    <script>
+        if (window.testRunner)
+            window.testRunner.waitUntilDone();
+
+        var range = document.createRange(); 
+        range.selectNode(document.body); 
+        window.getSelection().addRange(range);
+
+        function select() {
+            document.getElementById("foo").style.display = "none";
+            if (window.testRunner)
+                window.testRunner.notifyDone();
+        }
+
+        setTimeout(select, 0);
+    </script>
+</body>
+</html>
index f75a515..af6eddd 100644 (file)
@@ -1,3 +1,29 @@
+2014-07-28  Zalan Bujtas  <zalan@apple.com>
+
+        REGRESSION(r164133): Selection disappears after scrolling on nytimes.com
+        https://bugs.webkit.org/show_bug.cgi?id=135361
+
+        Reviewed by Ryosuke Niwa.
+
+        Ensure that when a RenderElement, part of the current selection is removed,
+        we recalculate and update the selection soon after layout.
+
+        Test: fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed.html
+
+        * editing/FrameSelection.cpp:
+        (WebCore::FrameSelection::setNeedsSelectionUpdate):
+        (WebCore::FrameSelection::didLayout): didLayout name reflects its functionality better.
+        (WebCore::FrameSelection::layoutDidChange): Deleted.
+        * editing/FrameSelection.h: : move some functions to private.
+        * page/FrameView.cpp:
+        (WebCore::FrameView::performPostLayoutTasks):
+        * rendering/RenderBlockFlow.cpp:
+        (WebCore::RenderBlockFlow::willBeDestroyed):
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::removeChildInternal):
+        * rendering/RenderInline.cpp:
+        (WebCore::RenderInline::willBeDestroyed):
+
 2014-07-28  Dean Jackson  <dino@apple.com>
 
         [Media iOS] Touching play button feels unresponsive
index 017acd3..fc5ce12 100644 (file)
@@ -352,6 +352,13 @@ static void updateSelectionByUpdatingLayoutOrStyle(Frame& frame)
 #endif
 }
 
+void FrameSelection::setNeedsSelectionUpdate()
+{
+    m_pendingSelectionUpdate = true;
+    if (RenderView* view = m_frame->contentRenderer())
+        view->clearSelection();
+}
+
 void FrameSelection::updateAndRevealSelection()
 {
     if (!m_pendingSelectionUpdate)
@@ -2141,7 +2148,7 @@ void FrameSelection::setShouldShowBlockCursor(bool shouldShowBlockCursor)
     updateAppearance();
 }
 
-void FrameSelection::layoutDidChange()
+void FrameSelection::didLayout()
 {
     setCaretRectNeedsUpdate();
     updateAndRevealSelection();
index a44d5bd..3a6ed69 100644 (file)
@@ -144,14 +144,13 @@ public:
 
     const VisibleSelection& selection() const { return m_selection; }
     void setSelection(const VisibleSelection&, SetSelectionOptions = defaultSetSelectionOptions(), CursorAlignOnScroll = AlignCursorOnScrollIfNeeded, TextGranularity = CharacterGranularity);
-    void updateAndRevealSelection();
-    void updateDataDetectorsForSelection();
     bool setSelectedRange(Range*, EAffinity, bool closeTyping);
     void selectAll();
     void clear();
     void prepareForDestruction();
 
-    void layoutDidChange();
+    void didLayout();
+    void setNeedsSelectionUpdate();
 
     bool contains(const LayoutPoint&);
 
@@ -272,6 +271,9 @@ public:
 private:
     enum EPositionType { START, END, BASE, EXTENT };
 
+    void updateAndRevealSelection();
+    void updateDataDetectorsForSelection();
+
     bool setSelectionWithoutUpdatingAppearance(const VisibleSelection&, SetSelectionOptions, CursorAlignOnScroll, TextGranularity);
 
     void respondToNodeModification(Node*, bool baseRemoved, bool extentRemoved, bool startRemoved, bool endRemoved);
index c3d0889..99b4632 100644 (file)
@@ -2816,7 +2816,7 @@ void FrameView::performPostLayoutTasks()
 
     m_postLayoutTasksTimer.stop();
 
-    frame().selection().layoutDidChange();
+    frame().selection().didLayout();
 
     if (m_nestedLayoutCount <= 1 && frame().document()->documentElement())
         fireLayoutRelatedMilestonesIfNeeded();
index a633673..e471b9d 100644 (file)
@@ -27,6 +27,7 @@
 #include "Editor.h"
 #include "FloatingObjects.h"
 #include "Frame.h"
+#include "FrameSelection.h"
 #include "HTMLElement.h"
 #include "HitTestLocation.h"
 #include "InlineTextBox.h"
@@ -162,10 +163,8 @@ void RenderBlockFlow::willBeDestroyed()
         if (firstRootBox()) {
             // We can't wait for RenderBox::destroy to clear the selection,
             // because by then we will have nuked the line boxes.
-            // FIXME: The FrameSelection should be responsible for this when it
-            // is notified of DOM mutations.
             if (isSelectionBorder())
-                view().clearSelection();
+                frame().selection().setNeedsSelectionUpdate();
 
             // If we are an anonymous block, then our line boxes might have children
             // that will outlast this block. In the non-anonymous block case those
index 251dd4f..62c9196 100644 (file)
@@ -31,6 +31,7 @@
 #include "CursorList.h"
 #include "EventHandler.h"
 #include "Frame.h"
+#include "FrameSelection.h"
 #include "HTMLElement.h"
 #include "HTMLNames.h"
 #include "FlowThreadController.h"
@@ -607,10 +608,8 @@ RenderObject* RenderElement::removeChildInternal(RenderObject& oldChild, NotifyC
 
     // If oldChild is the start or end of the selection, then clear the selection to
     // avoid problems of invalid pointers.
-    // FIXME: The FrameSelection should be responsible for this when it
-    // is notified of DOM mutations.
     if (!documentBeingDestroyed() && oldChild.isSelectionBorder())
-        view().clearSelection();
+        frame().selection().setNeedsSelectionUpdate();
 
     if (!documentBeingDestroyed() && notifyChildren == NotifyChildren)
         oldChild.willBeRemovedFromTree();
index 3028a4d..559b351 100644 (file)
@@ -25,6 +25,7 @@
 
 #include "Chrome.h"
 #include "FloatQuad.h"
+#include "FrameSelection.h"
 #include "GraphicsContext.h"
 #include "HitTestResult.h"
 #include "InlineElementBox.h"
@@ -93,10 +94,8 @@ void RenderInline::willBeDestroyed()
         if (firstLineBox()) {
             // We can't wait for RenderBoxModelObject::destroy to clear the selection,
             // because by then we will have nuked the line boxes.
-            // FIXME: The FrameSelection should be responsible for this when it
-            // is notified of DOM mutations.
             if (isSelectionBorder())
-                view().clearSelection();
+                frame().selection().setNeedsSelectionUpdate();
 
             // If line boxes are contained inside a root, that means we're an inline.
             // In that case, we need to remove all the line boxes so that the parent