Phantom focus/blur events fire on clicking between text input fields when listening...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Nov 2018 06:39:15 +0000 (06:39 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Nov 2018 06:39:15 +0000 (06:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179990

Reviewed by Tim Horton.

The bug was caused by TemporarySelectionChange which is used by TextIndicator::createWithRange
to set and restore the selection putting the focus on the newly mouse-down'ed input element
and restoring the focus back to the input element which originally had the focus immediately.

Fixed the bug by avoiding to set the focus since only selection highlights need to be updated here.
Also made TemporarySelectionOption an enum class.

Unfortunately, no new tests since force click testing is broken :( See <rdar://problem/31301721>.

* editing/Editor.cpp:
(WebCore::TemporarySelectionChange::TemporarySelectionChange):
(WebCore::TemporarySelectionChange::~TemporarySelectionChange):
(WebCore::TemporarySelectionChange::setSelection): Extracted. Fixed the bug by adding
FrameSelection::DoNotSetFocus to the option when TemporarySelectionOption::DoNotSetFocus is set.
* editing/Editor.h:
* page/DragController.cpp:
(WebCore::DragController::performDragOperation):
* page/TextIndicator.cpp:
(WebCore::TextIndicator::createWithRange): Set TemporarySelectionOption::DoNotSetFocus.

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

Source/WebCore/ChangeLog
Source/WebCore/editing/Editor.cpp
Source/WebCore/editing/Editor.h
Source/WebCore/page/DragController.cpp
Source/WebCore/page/TextIndicator.cpp

index 4a0c820..fa03b3f 100644 (file)
@@ -1,3 +1,30 @@
+2018-11-21  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Phantom focus/blur events fire on clicking between text input fields when listening with addEventListener
+        https://bugs.webkit.org/show_bug.cgi?id=179990
+
+        Reviewed by Tim Horton.
+
+        The bug was caused by TemporarySelectionChange which is used by TextIndicator::createWithRange
+        to set and restore the selection putting the focus on the newly mouse-down'ed input element
+        and restoring the focus back to the input element which originally had the focus immediately.
+
+        Fixed the bug by avoiding to set the focus since only selection highlights need to be updated here.
+        Also made TemporarySelectionOption an enum class.
+
+        Unfortunately, no new tests since force click testing is broken :( See <rdar://problem/31301721>.
+
+        * editing/Editor.cpp:
+        (WebCore::TemporarySelectionChange::TemporarySelectionChange):
+        (WebCore::TemporarySelectionChange::~TemporarySelectionChange):
+        (WebCore::TemporarySelectionChange::setSelection): Extracted. Fixed the bug by adding
+        FrameSelection::DoNotSetFocus to the option when TemporarySelectionOption::DoNotSetFocus is set.
+        * editing/Editor.h:
+        * page/DragController.cpp:
+        (WebCore::DragController::performDragOperation):
+        * page/TextIndicator.cpp:
+        (WebCore::TextIndicator::createWithRange): Set TemporarySelectionOption::DoNotSetFocus.
+
 2018-11-21  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         [Cocoa] [WebKit2] Add support for replacing find-in-page text matches
index 023ed19..2b65bdd 100644 (file)
@@ -206,35 +206,43 @@ TemporarySelectionChange::TemporarySelectionChange(Frame& frame, std::optional<V
 #endif
 {
 #if PLATFORM(IOS_FAMILY)
-    if (options & TemporarySelectionOptionEnableAppearanceUpdates)
+    if (options & TemporarySelectionOption::EnableAppearanceUpdates)
         frame.selection().setUpdateAppearanceEnabled(true);
 #endif
 
-    if (options & TemporarySelectionOptionIgnoreSelectionChanges)
+    if (options & TemporarySelectionOption::IgnoreSelectionChanges)
         frame.editor().setIgnoreSelectionChanges(true);
 
     if (temporarySelection) {
         m_selectionToRestore = frame.selection().selection();
-        frame.selection().setSelection(temporarySelection.value());
+        setSelection(temporarySelection.value());
     }
 }
 
 TemporarySelectionChange::~TemporarySelectionChange()
 {
     if (m_selectionToRestore)
-        m_frame->selection().setSelection(m_selectionToRestore.value());
+        setSelection(m_selectionToRestore.value());
 
-    if (m_options & TemporarySelectionOptionIgnoreSelectionChanges) {
-        auto revealSelection = m_options & TemporarySelectionOptionRevealSelection ? Editor::RevealSelection::Yes : Editor::RevealSelection::No;
+    if (m_options & TemporarySelectionOption::IgnoreSelectionChanges) {
+        auto revealSelection = m_options & TemporarySelectionOption::RevealSelection ? Editor::RevealSelection::Yes : Editor::RevealSelection::No;
         m_frame->editor().setIgnoreSelectionChanges(m_wasIgnoringSelectionChanges, revealSelection);
     }
 
 #if PLATFORM(IOS_FAMILY)
-    if (m_options & TemporarySelectionOptionEnableAppearanceUpdates)
+    if (m_options & TemporarySelectionOption::EnableAppearanceUpdates)
         m_frame->selection().setUpdateAppearanceEnabled(m_appearanceUpdatesWereEnabled);
 #endif
 }
 
+void TemporarySelectionChange::setSelection(const VisibleSelection& selection)
+{
+    auto options = FrameSelection::defaultSetSelectionOptions();
+    if (m_options & TemporarySelectionOption::DoNotSetFocus)
+        options.add(FrameSelection::DoNotSetFocus);
+    m_frame->selection().setSelection(selection, options);
+}
+
 // When an event handler has moved the selection outside of a text control
 // we should use the target control's selection for this editing operation.
 VisibleSelection Editor::selectionForCommand(Event* event)
index 0ddea82..e8ffcc8 100644 (file)
@@ -102,15 +102,15 @@ enum class MailBlockquoteHandling {
 class HTMLAttachmentElement;
 #endif
 
-enum TemporarySelectionOption : uint8_t {
-    // Scroll to reveal the selection.
-    TemporarySelectionOptionRevealSelection = 1 << 0,
+enum class TemporarySelectionOption : uint8_t {
+    RevealSelection = 1 << 0,
+    DoNotSetFocus = 1 << 1,
 
     // Don't propagate selection changes to the client layer.
-    TemporarySelectionOptionIgnoreSelectionChanges = 1 << 1,
+    IgnoreSelectionChanges = 1 << 2,
 
     // Force the render tree to update selection state. Only respected on iOS.
-    TemporarySelectionOptionEnableAppearanceUpdates = 1 << 2
+    EnableAppearanceUpdates = 1 << 3,
 };
 
 class TemporarySelectionChange {
@@ -119,6 +119,8 @@ public:
     ~TemporarySelectionChange();
 
 private:
+    void setSelection(const VisibleSelection&);
+
     Ref<Frame> m_frame;
     OptionSet<TemporarySelectionOption> m_options;
     bool m_wasIgnoringSelectionChanges;
index 02d4d54..7562136 100644 (file)
@@ -251,7 +251,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);
+    TemporarySelectionChange ignoreSelectionChanges(m_page.focusController().focusedOrMainFrame(), std::nullopt, TemporarySelectionOption::IgnoreSelectionChanges);
 
     m_documentUnderMouse = m_page.mainFrame().documentAtPoint(dragData.clientPosition());
 
index 49924b2..6163403 100644 (file)
@@ -77,9 +77,10 @@ RefPtr<TextIndicator> TextIndicator::createWithRange(const Range& range, TextInd
 
     VisibleSelection oldSelection = frame->selection().selection();
     OptionSet<TemporarySelectionOption> temporarySelectionOptions;
+    temporarySelectionOptions.add(TemporarySelectionOption::DoNotSetFocus);
 #if PLATFORM(IOS_FAMILY)
-    temporarySelectionOptions.add(TemporarySelectionOptionIgnoreSelectionChanges);
-    temporarySelectionOptions.add(TemporarySelectionOptionEnableAppearanceUpdates);
+    temporarySelectionOptions.add(TemporarySelectionOption::IgnoreSelectionChanges);
+    temporarySelectionOptions.add(TemporarySelectionOption::EnableAppearanceUpdates);
 #endif
     TemporarySelectionChange selectionChange(*frame, { range }, temporarySelectionOptions);