Skip to content
Permalink
Browse files
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.


Canonical link: https://commits.webkit.org/206610@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238440 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
rniwa committed Nov 22, 2018
1 parent 933424d commit c023b680d7c6541c3d65dce6818dde3fb8df692b
Showing with 53 additions and 15 deletions.
  1. +27 −0 Source/WebCore/ChangeLog
  2. +15 −7 Source/WebCore/editing/Editor.cpp
  3. +7 −5 Source/WebCore/editing/Editor.h
  4. +1 −1 Source/WebCore/page/DragController.cpp
  5. +3 −2 Source/WebCore/page/TextIndicator.cpp
@@ -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
@@ -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)
@@ -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 @@ class TemporarySelectionChange {
~TemporarySelectionChange();

private:
void setSelection(const VisibleSelection&);

Ref<Frame> m_frame;
OptionSet<TemporarySelectionOption> m_options;
bool m_wasIgnoringSelectionChanges;
@@ -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());

@@ -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);

0 comments on commit c023b68

Please sign in to comment.