Title: [171718] trunk
Revision
171718
Author
za...@apple.com
Date
2014-07-28 18:45:54 -0700 (Mon, 28 Jul 2014)

Log Message

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.

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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (171717 => 171718)


--- trunk/LayoutTests/ChangeLog	2014-07-29 01:03:08 UTC (rev 171717)
+++ trunk/LayoutTests/ChangeLog	2014-07-29 01:45:54 UTC (rev 171718)
@@ -1,3 +1,16 @@
+2014-07-28  Zalan Bujtas  <za...@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  <akl...@apple.com>
 
         REGRESSION (r160806): CSS zoom property doesn't work on anything inside anchors.

Added: trunk/LayoutTests/fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed-expected.html (0 => 171718)


--- trunk/LayoutTests/fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed-expected.html	2014-07-29 01:45:54 UTC (rev 171718)
@@ -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>

Added: trunk/LayoutTests/fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed.html (0 => 171718)


--- trunk/LayoutTests/fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed.html	                        (rev 0)
+++ trunk/LayoutTests/fast/dynamic/selection-gets-cleared-when-part-of-it-gets-removed.html	2014-07-29 01:45:54 UTC (rev 171718)
@@ -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>

Modified: trunk/Source/WebCore/ChangeLog (171717 => 171718)


--- trunk/Source/WebCore/ChangeLog	2014-07-29 01:03:08 UTC (rev 171717)
+++ trunk/Source/WebCore/ChangeLog	2014-07-29 01:45:54 UTC (rev 171718)
@@ -1,3 +1,29 @@
+2014-07-28  Zalan Bujtas  <za...@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  <d...@apple.com>
 
         [Media iOS] Touching play button feels unresponsive

Modified: trunk/Source/WebCore/editing/FrameSelection.cpp (171717 => 171718)


--- trunk/Source/WebCore/editing/FrameSelection.cpp	2014-07-29 01:03:08 UTC (rev 171717)
+++ trunk/Source/WebCore/editing/FrameSelection.cpp	2014-07-29 01:45:54 UTC (rev 171718)
@@ -352,6 +352,13 @@
 #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 @@
     updateAppearance();
 }
 
-void FrameSelection::layoutDidChange()
+void FrameSelection::didLayout()
 {
     setCaretRectNeedsUpdate();
     updateAndRevealSelection();

Modified: trunk/Source/WebCore/editing/FrameSelection.h (171717 => 171718)


--- trunk/Source/WebCore/editing/FrameSelection.h	2014-07-29 01:03:08 UTC (rev 171717)
+++ trunk/Source/WebCore/editing/FrameSelection.h	2014-07-29 01:45:54 UTC (rev 171718)
@@ -144,14 +144,13 @@
 
     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 @@
 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);

Modified: trunk/Source/WebCore/page/FrameView.cpp (171717 => 171718)


--- trunk/Source/WebCore/page/FrameView.cpp	2014-07-29 01:03:08 UTC (rev 171717)
+++ trunk/Source/WebCore/page/FrameView.cpp	2014-07-29 01:45:54 UTC (rev 171718)
@@ -2816,7 +2816,7 @@
 
     m_postLayoutTasksTimer.stop();
 
-    frame().selection().layoutDidChange();
+    frame().selection().didLayout();
 
     if (m_nestedLayoutCount <= 1 && frame().document()->documentElement())
         fireLayoutRelatedMilestonesIfNeeded();

Modified: trunk/Source/WebCore/rendering/RenderBlockFlow.cpp (171717 => 171718)


--- trunk/Source/WebCore/rendering/RenderBlockFlow.cpp	2014-07-29 01:03:08 UTC (rev 171717)
+++ trunk/Source/WebCore/rendering/RenderBlockFlow.cpp	2014-07-29 01:45:54 UTC (rev 171718)
@@ -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 @@
         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

Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (171717 => 171718)


--- trunk/Source/WebCore/rendering/RenderElement.cpp	2014-07-29 01:03:08 UTC (rev 171717)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp	2014-07-29 01:45:54 UTC (rev 171718)
@@ -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 @@
 
     // 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();

Modified: trunk/Source/WebCore/rendering/RenderInline.cpp (171717 => 171718)


--- trunk/Source/WebCore/rendering/RenderInline.cpp	2014-07-29 01:03:08 UTC (rev 171717)
+++ trunk/Source/WebCore/rendering/RenderInline.cpp	2014-07-29 01:45:54 UTC (rev 171718)
@@ -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 @@
         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
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to