Changeset 238440 in webkit

Timestamp:

Nov 21, 2018 10:39:15 PM (5 years ago)

Author:

rniwa@webkit.org

Message:

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.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• editing/Editor.cpp (modified) (2 diffs)
• editing/Editor.h (modified) (2 diffs)
• page/DragController.cpp (modified) (1 diff)
• page/TextIndicator.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/editing/Editor.cpp

@@ -207 +207 @@
 {
 #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());
     }
 }
@@ -223 +223 @@
 {
     if (m_selectionToRestore)
-        m_frame->selection().setSelection(m_selectionToRestore.value());
-
-    if (m_options & TemporarySelectionOptionIgnoreSelectionChanges) {
-        auto revealSelection = m_options & TemporarySelectionOptionRevealSelection ? Editor::RevealSelection::Yes : Editor::RevealSelection::No;
+        setSelection(m_selectionToRestore.value());
+
+    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);
 }
 

trunk/Source/WebCore/editing/Editor.h

@@ -103 +103 @@
 #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,
 };
 
@@ -120 +120 @@
 
 private:
+    void setSelection(const VisibleSelection&);
+
     Ref<Frame> m_frame;
     OptionSet<TemporarySelectionOption> m_options;

trunk/Source/WebCore/page/DragController.cpp

@@ -252 +252 @@
 {
     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());

trunk/Source/WebCore/page/TextIndicator.cpp

@@ -78 +78 @@
     VisibleSelection oldSelection = frame->selection().selection();
     OptionSet<TemporarySelectionOption> temporarySelectionOptions;
-#if PLATFORM(IOS_FAMILY)
-    temporarySelectionOptions.add(TemporarySelectionOptionIgnoreSelectionChanges);
-    temporarySelectionOptions.add(TemporarySelectionOptionEnableAppearanceUpdates);
+    temporarySelectionOptions.add(TemporarySelectionOption::DoNotSetFocus);
+#if PLATFORM(IOS_FAMILY)
+    temporarySelectionOptions.add(TemporarySelectionOption::IgnoreSelectionChanges);
+    temporarySelectionOptions.add(TemporarySelectionOption::EnableAppearanceUpdates);
 #endif
     TemporarySelectionChange selectionChange(*frame, { range }, temporarySelectionOptions);