[iOS WK2] 5 DataInteractionTests are failing: observed selection rects after dropping...

https://bugs.webkit.org/show_bug.cgi?id=174769
<rdar://problem/33478864>

Reviewed by Tim Horton.

Source/WebCore:

These tests began failing after r219541, due to a change in behavior of whether or not we call
setIgnoreSelectionChanges(false, RevealSelection::No) or setIgnoreSelectionChanges(false, RevealSelection::Yes)
when performing a text editing drop.

Before r219541, we would not reveal the selection when performing an edit drag operation. This is because in
WebPage::performDragControllerAction, we would begin ignoring selection changes by calling
setIgnoreSelectionChanges(true). However, while taking a text indicator snapshot, we would stop ignoring
selection in TextIndicator::createWithRange due to us calling setIgnoreSelectionChanges(false,
RevealSelection::No) at the end of the function. Then, when we return to the scope of
WebPage::performDragControllerAction and try to setIgnoreSelectionChanges(false), this is a no-op because we've
already stopped ignoring selection changes.

After r219541, switching to using TemporarySelectionChange means that TextIndicator::createWithRange now means
we respect whether or not we were already ignoring selection before taking the snapshot, so we won't always
setIgnoreSelectionChanges(false) at the end. This means that selection changes will now be correctly ignored
when performing a drag operation, but this also means that we'll try to reveal the selection, since
WebPage::performDragControllerAction calls setIgnoreSelectionChanges(false), for which RevealSelection::Yes
is used by default.

Revealing the selection in WebPage::performDragControllerAction was unintended in the first place, so we should
revert to calling setIgnoreSelectionChanges(false, RevealSelection::No). To ensure this, we adopt
TemporarySelectionChange here and pass only TemporarySelectionOptionIgnoreSelectionChanges, so that we won't
additionally try to reveal selection after the drop. This is consistent with behavior prior to macOS 10.13 and
iOS 11. Additionally, this patch also moves the call to ignore selection change from WebKit into WebCore, so
that whether we ignore selection is consistent across both WebKit1 and WebKit2.

* page/DragController.cpp:
(WebCore::DragController::performDragOperation):

Source/WebKit:

Fixes several API tests in the DataInteractionTests suite. See Source/WebCore/ChangeLog for more details.

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::performDragControllerAction):

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index bf9239eda663385b2acf7eddac6b3ee222384ee0..42eb26ba69a8886e8eef1abc57010c87470cd4d5 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,40 @@
+2017-07-23  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS WK2] 5 DataInteractionTests are failing: observed selection rects after dropping don't match expected values
+        https://bugs.webkit.org/show_bug.cgi?id=174769
+        <rdar://problem/33478864>
+
+        Reviewed by Tim Horton.
+
+        These tests began failing after r219541, due to a change in behavior of whether or not we call
+        setIgnoreSelectionChanges(false, RevealSelection::No) or setIgnoreSelectionChanges(false, RevealSelection::Yes)
+        when performing a text editing drop.
+
+        Before r219541, we would not reveal the selection when performing an edit drag operation. This is because in
+        WebPage::performDragControllerAction, we would begin ignoring selection changes by calling
+        setIgnoreSelectionChanges(true). However, while taking a text indicator snapshot, we would stop ignoring
+        selection in TextIndicator::createWithRange due to us calling setIgnoreSelectionChanges(false,
+        RevealSelection::No) at the end of the function. Then, when we return to the scope of
+        WebPage::performDragControllerAction and try to setIgnoreSelectionChanges(false), this is a no-op because we've
+        already stopped ignoring selection changes.
+
+        After r219541, switching to using TemporarySelectionChange means that TextIndicator::createWithRange now means
+        we respect whether or not we were already ignoring selection before taking the snapshot, so we won't always
+        setIgnoreSelectionChanges(false) at the end. This means that selection changes will now be correctly ignored
+        when performing a drag operation, but this also means that we'll try to reveal the selection, since
+        WebPage::performDragControllerAction calls setIgnoreSelectionChanges(false), for which RevealSelection::Yes
+        is used by default.
+
+        Revealing the selection in WebPage::performDragControllerAction was unintended in the first place, so we should
+        revert to calling setIgnoreSelectionChanges(false, RevealSelection::No). To ensure this, we adopt
+        TemporarySelectionChange here and pass only TemporarySelectionOptionIgnoreSelectionChanges, so that we won't
+        additionally try to reveal selection after the drop. This is consistent with behavior prior to macOS 10.13 and
+        iOS 11. Additionally, this patch also moves the call to ignore selection change from WebKit into WebCore, so
+        that whether we ignore selection is consistent across both WebKit1 and WebKit2.
+
+        * page/DragController.cpp:
+        (WebCore::DragController::performDragOperation):
+
 2017-07-23  Chris Dumez  <cdumez@apple.com>
 
         Drop ExceptionCodeDescription class

diff --git a/Source/WebCore/page/DragController.cpp b/Source/WebCore/page/DragController.cpp
index a4479b01cd821612e9b250a9afe0f26908b844fe..a13c30eefa4d126260c4dd02811e5d09a8f96d52 100644 (file)
--- a/Source/WebCore/page/DragController.cpp
+++ b/Source/WebCore/page/DragController.cpp
@@ -46,6 +46,7 @@
 #include "ElementAncestorIterator.h"
 #include "EventHandler.h"
 #include "FloatRect.h"
+#include "FocusController.h"
 #include "FrameLoadRequest.h"
 #include "FrameLoader.h"
 #include "FrameSelection.h"
@@ -258,6 +259,7 @@ inline static bool dragIsHandledByDocument(DragController::DragHandlingMethod dr
 bool DragController::performDragOperation(const DragData& dragData)
 {
     SetForScope<bool> isPerformingDrop(m_isPerformingDrop, true);
+    TemporarySelectionChange ignoreSelectionChanges(m_page.focusController().focusedOrMainFrame(), std::nullopt, TemporarySelectionOptionIgnoreSelectionChanges);
 
     m_documentUnderMouse = m_page.mainFrame().documentAtPoint(dragData.clientPosition());
 

diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index b97c1f34381fe5672318c3b21609b2ebc2dc95ad..dad0cc347adb008ece393d0fbfb3c3a6fec22100 100644 (file)
--- a/Source/WebKit/ChangeLog
+++ b/Source/WebKit/ChangeLog
@@ -1,3 +1,16 @@
+2017-07-23  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS WK2] 5 DataInteractionTests are failing: observed selection rects after dropping don't match expected values
+        https://bugs.webkit.org/show_bug.cgi?id=174769
+        <rdar://problem/33478864>
+
+        Reviewed by Tim Horton.
+
+        Fixes several API tests in the DataInteractionTests suite. See Source/WebCore/ChangeLog for more details.
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::performDragControllerAction):
+
 2017-07-20  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         WebDriver: implement page load timeout

diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
index 32d866fb8a71adeaa3139251852384f78f1e872b..a20f1d5c064d61b5e29ad9edd5d1e04efbc0d57b 100644 (file)
--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
@@ -3616,10 +3616,7 @@ void WebPage::performDragControllerAction(uint64_t action, const WebCore::DragDa
                 m_pendingDropExtensionsForFileUpload.append(extension);
         }
 
-        auto& frame = m_page->focusController().focusedOrMainFrame();
-        frame.editor().setIgnoreSelectionChanges(true);
         bool handled = m_page->dragController().performDragOperation(dragData);
-        frame.editor().setIgnoreSelectionChanges(false);
 
         // If we started loading a local file, the sandbox extension tracker would have adopted this
         // pending drop sandbox extension. If not, we'll play it safe and clear it.