Coalesce selectionchange events

https://bugs.webkit.org/show_bug.cgi?id=271117 Reviewed by Chris Dumez. Implement the spec change to avoid queuing a task to fire a selectionchange event when there is already a task scheduled to do that for the target: w3c/selection-api#172 Also update the relevant web platform tests: web-platform-tests/wpt#45145 * LayoutTests/fast/events/selectionchange-iframe-expected.txt: * LayoutTests/fast/events/selectionchange-user-initiated-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/selection/onselectionchange-on-distinct-text-controls-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/selection/onselectionchange-on-distinct-text-controls.html: Added. * LayoutTests/imported/w3c/web-platform-tests/selection/onselectionchange-on-document-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/selection/onselectionchange-on-document.html: Added. * LayoutTests/imported/w3c/web-platform-tests/selection/textcontrols/selectionchange.html: * Source/WebCore/editing/FrameSelection.cpp: (WebCore::FrameSelection::setSelectionWithoutUpdatingAppearance): * Source/WebCore/editing/FrameSelection.h: * Source/WebCore/html/HTMLTextFormControlElement.cpp: (WebCore::HTMLTextFormControlElement::HTMLTextFormControlElement): (WebCore::HTMLTextFormControlElement::scheduleSelectionChangeEvent): (WebCore::HTMLTextFormControlElement::scheduleSelectEvent): * Source/WebCore/html/HTMLTextFormControlElement.h: (WebCore::HTMLTextFormControlElement::hasScheduledSelectionChangeEvent const): (WebCore::HTMLTextFormControlElement::setHasScheduledSelectionChangeEvent): Canonical link: https://commits.webkit.org/276388@main
rniwa · Mar 20, 2024 · 9745969 · 9745969
1 parent 0881857
commit 9745969
Show file tree

Hide file tree

Showing 11 changed files with 172 additions and 26 deletions.
diff --git a/LayoutTests/fast/events/selectionchange-iframe-expected.txt b/LayoutTests/fast/events/selectionchange-iframe-expected.txt
@@ -2,4 +2,4 @@ This tests that changing selection in an iframe does not fire selectionchange ev
 
 PASS: selecting all of parent's document body fired selection change events from parent
 PASS: selecting all of parent's document body fired selection change events from child
-PASS: selecting "rocks" of "WebKit rocks" in the parent fired selection change events from parent, parent
+PASS: selecting "rocks" of "WebKit rocks" in the parent fired selection change events from parent
diff --git a/LayoutTests/fast/events/selectionchange-user-initiated-expected.txt b/LayoutTests/fast/events/selectionchange-user-initiated-expected.txt
@@ -14,7 +14,7 @@ PASS: keyup
 PASS: selectionchange
 PASS: mousedown
 PASS: selectionchange
-PASS: selectionchange
+OPTIONAL: selectionchange
 PASS: mouseup
 PASS: click
 PASS: keydown

diff --git a/...w3c/web-platform-tests/selection/onselectionchange-on-distinct-text-controls-expected.txt b/...w3c/web-platform-tests/selection/onselectionchange-on-distinct-text-controls-expected.txt
@@ -0,0 +1,5 @@
+
+
+PASS selectionchange event on each input element fires independently
+PASS selectionchange event on each textarea element fires independently
+
diff --git a/...mported/w3c/web-platform-tests/selection/onselectionchange-on-distinct-text-controls.html b/...mported/w3c/web-platform-tests/selection/onselectionchange-on-distinct-text-controls.html
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel="help" href="https://w3c.github.io/selection-api/#selectionchange-event">
+<script src="../resources/testharness.js"></script>
+<script src="../resources/testharnessreport.js"></script>
+<body>
+<input id="input1" value="hello">
+<input id="input2" value="world">
+<textarea id="textarea1">hello</textarea>
+<textarea id="textarea2">world</textarea>
+<script>
+
+promise_test(() => {
+  return (async function() {
+    let selectionChangeCount1 = 0;
+    let selectionChangeCount2 = 0;
+    input1.addEventListener("selectionchange", () => ++selectionChangeCount1);
+    input2.addEventListener("selectionchange", () => ++selectionChangeCount2);
+    input1.setSelectionRange(1, 2);
+    input1.setSelectionRange(2, 3);
+    input2.setSelectionRange(1, 3);
+    assert_equals(selectionChangeCount1, 0);
+    assert_equals(selectionChangeCount2, 0);
+    await new Promise((resolve) => setTimeout(resolve, 0));
+    assert_equals(selectionChangeCount1, 1);
+    assert_equals(selectionChangeCount2, 1);
+  })();
+}, "selectionchange event on each input element fires independently");
+
+promise_test(() => {
+  return (async function() {
+    let selectionChangeCount1 = 0;
+    let selectionChangeCount2 = 0;
+    textarea1.addEventListener("selectionchange", () => ++selectionChangeCount1);
+    textarea2.addEventListener("selectionchange", () => ++selectionChangeCount2);
+    textarea1.setSelectionRange(1, 2);
+    textarea1.setSelectionRange(2, 3);
+    textarea2.setSelectionRange(1, 3);
+    assert_equals(selectionChangeCount1, 0);
+    assert_equals(selectionChangeCount2, 0);
+    await new Promise((resolve) => setTimeout(resolve, 0));
+    assert_equals(selectionChangeCount1, 1);
+    assert_equals(selectionChangeCount2, 1);
+  })();
+}, "selectionchange event on each textarea element fires independently");
+
+
+</script>
diff --git a/...ests/imported/w3c/web-platform-tests/selection/onselectionchange-on-document-expected.txt b/...ests/imported/w3c/web-platform-tests/selection/onselectionchange-on-document-expected.txt
@@ -0,0 +1,8 @@
+
+
+
+PASS selectionchange event on document fires
+PASS selectionchange event on document fires once
+PASS task to fire selectionchange event gets queued each time selection is mutated
+PASS has scheduled selectionchange event is set to false at the beginning of a task to fire selectionchange event
+
diff --git a/LayoutTests/imported/w3c/web-platform-tests/selection/onselectionchange-on-document.html b/LayoutTests/imported/w3c/web-platform-tests/selection/onselectionchange-on-document.html
@@ -0,0 +1,73 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel="help" href="https://w3c.github.io/selection-api/#selectionchange-event">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<body>
+<div id="container"><br><br></div>
+<script>
+
+promise_test(() => {
+  return new Promise(resolve => {
+    let didFireSelectionChangeEvent = false;
+    document.addEventListener("selectionchange", () => { didFireSelectionChangeEvent = true; resolve(); }, {once: true});
+    getSelection().setPosition(container, 0);
+    assert_false(didFireSelectionChangeEvent);
+  });
+}, "selectionchange event on document fires");
+
+promise_test(() => {
+  return (async function() {
+    let selectionChangeCount = 0;
+    document.addEventListener("selectionchange", () => ++selectionChangeCount);
+    container.innerHTML = '<span><br></span><span><br></span>';
+    getSelection().setPosition(container, 0);
+    assert_equals(selectionChangeCount, 0);
+    getSelection().setPosition(container, 2);
+    assert_equals(selectionChangeCount, 0);
+    await new Promise((resolve) => setTimeout(resolve, 0));
+    assert_equals(selectionChangeCount, 1);
+  })();
+}, "selectionchange event on document fires once");
+
+promise_test(() => {
+  return (async function() {
+    let selectionChangeCount = 0;
+    document.addEventListener("selectionchange", () => ++selectionChangeCount);
+    container.innerHTML = '<span><br></span><span><br></span>';
+    getSelection().setPosition(container, 0);
+    assert_equals(selectionChangeCount, 0);
+    getSelection().setPosition(container, 2);
+    assert_equals(selectionChangeCount, 0);
+    await new Promise((resolve) => setTimeout(resolve, 0));
+    assert_equals(selectionChangeCount, 1);
+    getSelection().setPosition(container, 0);
+    assert_equals(selectionChangeCount, 1);
+    getSelection().setPosition(container, 2);
+    assert_equals(selectionChangeCount, 1);
+    await new Promise((resolve) => setTimeout(resolve, 0));
+    assert_equals(selectionChangeCount, 2);
+  })();
+}, "task to fire selectionchange event gets queued each time selection is mutated");
+
+promise_test(() => {
+  return (async function() {
+    let selectionChangeCount = 0;
+    document.addEventListener("selectionchange", () => {
+      if (!selectionChangeCount) {
+        getSelection().setPosition(container, 2);
+        getSelection().setPosition(container, 0);
+      }
+      ++selectionChangeCount;
+    });
+    container.innerHTML = '<b><br></b><b><br></b>';
+    getSelection().setPosition(container, 0);
+    assert_equals(selectionChangeCount, 0);
+    await new Promise((resolve) => setTimeout(resolve, 0));
+    assert_equals(selectionChangeCount, 1);
+    await new Promise((resolve) => setTimeout(resolve, 0));
+    assert_equals(selectionChangeCount, 2);
+  })();
+}, "has scheduled selectionchange event is set to false at the beginning of a task to fire selectionchange event");
+
+</script>
diff --git a/LayoutTests/imported/w3c/web-platform-tests/selection/textcontrols/selectionchange.html b/LayoutTests/imported/w3c/web-platform-tests/selection/textcontrols/selectionchange.html
@@ -184,7 +184,7 @@
       target.setRangeText("foo", 2, 6);
 
       await data.assert_empty_spin();
-      assert_equals(collector.events.length, 2);
+      assert_equals(collector.events.length, 1);
     }, `Calling setRangeText() after select() on ${name}`);
 
     promise_test(async () => {
@@ -196,7 +196,7 @@
       target.setRangeText("", 10, 12);
 
       await data.assert_empty_spin();
-      assert_equals(collector.events.length, 4);
+      assert_equals(collector.events.length, 1);
     }, `Calling setRangeText() repeatedly on ${name}`);
 
     promise_test(async () => {

diff --git a/Source/WebCore/editing/FrameSelection.cpp b/Source/WebCore/editing/FrameSelection.cpp
@@ -436,14 +436,15 @@ bool FrameSelection::setSelectionWithoutUpdatingAppearance(const VisibleSelectio
     document->editor().respondToChangedSelection(oldSelection, options);
 
     if (shouldScheduleSelectionChangeEvent) {
-        if (textControl) {
-            document->eventLoop().queueTask(TaskSource::UserInteraction, [textControl = GCReachableRef { *textControl }] {
-                textControl->dispatchEvent(Event::create(eventNames().selectionchangeEvent, Event::CanBubble::Yes, Event::IsCancelable::No));
-            });
-        } else {
+        if (textControl)
+            textControl->scheduleSelectionChangeEvent();
+        else if (!m_hasScheduledSelectionChangeEventOnDocument) {
+            m_hasScheduledSelectionChangeEventOnDocument = true;
             document->eventLoop().queueTask(TaskSource::UserInteraction, [weakDocument = WeakPtr { document.get() }] {
-                if (RefPtr document = weakDocument.get())
+                if (RefPtr document = weakDocument.get()) {
+                    document->selection().m_hasScheduledSelectionChangeEventOnDocument = false;
                     document->dispatchEvent(Event::create(eventNames().selectionchangeEvent, Event::CanBubble::No, Event::IsCancelable::No));
+                }
             });
         }
     }

diff --git a/Source/WebCore/editing/FrameSelection.h b/Source/WebCore/editing/FrameSelection.h
@@ -366,6 +366,7 @@ class FrameSelection final : private CaretBase, public CaretAnimationClient, pub
     bool m_shouldShowBlockCursor : 1;
     bool m_pendingSelectionUpdate : 1;
     bool m_alwaysAlignCursorOnScrollWhenRevealingSelection : 1;
+    bool m_hasScheduledSelectionChangeEventOnDocument : 1 { false };
 
 #if PLATFORM(IOS_FAMILY)
     bool m_updateAppearanceEnabled : 1;

diff --git a/Source/WebCore/html/HTMLTextFormControlElement.cpp b/Source/WebCore/html/HTMLTextFormControlElement.cpp
@@ -75,11 +75,6 @@ static Position positionForIndex(TextControlInnerTextElement*, unsigned);
 HTMLTextFormControlElement::HTMLTextFormControlElement(const QualifiedName& tagName, Document& document, HTMLFormElement* form)
     : HTMLFormControlElement(tagName, document, form)
     , m_cachedSelectionDirection(document.frame() && document.frame()->editor().behavior().shouldConsiderSelectionAsDirectional() ? SelectionHasForwardDirection : SelectionHasNoDirection)
-    , m_lastChangeWasUserEdit(false)
-    , m_isPlaceholderVisible(false)
-    , m_canShowPlaceholder(true)
-    , m_cachedSelectionStart(0)
-    , m_cachedSelectionEnd(0)
 {
 }
 
@@ -574,10 +569,22 @@ bool HTMLTextFormControlElement::selectionChanged(bool shouldFireSelectEvent)
     return previousSelectionStart != m_cachedSelectionStart || previousSelectionEnd != m_cachedSelectionEnd;
 }
 
+void HTMLTextFormControlElement::scheduleSelectionChangeEvent()
+{
+    if (m_hasScheduledSelectionChangeEvent)
+        return;
+
+    m_hasScheduledSelectionChangeEvent = true;
+    document().eventLoop().queueTask(TaskSource::UserInteraction, [textControl = GCReachableRef { *this }] {
+        textControl->m_hasScheduledSelectionChangeEvent = false;
+        textControl->dispatchEvent(Event::create(eventNames().selectionchangeEvent, Event::CanBubble::Yes, Event::IsCancelable::No));
+    });
+}
+
 void HTMLTextFormControlElement::scheduleSelectEvent()
 {
     queueTaskToDispatchEvent(TaskSource::UserInteraction, Event::create(eventNames().selectEvent, Event::CanBubble::Yes, Event::IsCancelable::No));
-    queueTaskToDispatchEvent(TaskSource::UserInteraction, Event::create(eventNames().selectionchangeEvent, Event::CanBubble::Yes, Event::IsCancelable::No));
+    scheduleSelectionChangeEvent();
 }
 
 void HTMLTextFormControlElement::attributeChanged(const QualifiedName& name, const AtomString& oldValue, const AtomString& newValue, AttributeModificationReason attributeModificationReason)

diff --git a/Source/WebCore/html/HTMLTextFormControlElement.h b/Source/WebCore/html/HTMLTextFormControlElement.h
@@ -83,6 +83,8 @@ class HTMLTextFormControlElement : public HTMLFormControlElement {
     void setSelectionRange(unsigned start, unsigned end, const String& direction, const AXTextStateChangeIntent& = AXTextStateChangeIntent(), ForBindings = ForBindings::No);
     WEBCORE_EXPORT bool setSelectionRange(unsigned start, unsigned end, TextFieldSelectionDirection = SelectionHasNoDirection, SelectionRevealMode = SelectionRevealMode::DoNotReveal, const AXTextStateChangeIntent& = AXTextStateChangeIntent(), ForBindings = ForBindings::No);
 
+    void scheduleSelectionChangeEvent();
+
     TextFieldSelectionDirection computeSelectionDirection() const;
 
     std::optional<SimpleRange> selection() const;
@@ -170,21 +172,22 @@ class HTMLTextFormControlElement : public HTMLFormControlElement {
     bool placeholderShouldBeVisible() const;
 
     unsigned m_cachedSelectionDirection : 2;
-    unsigned m_lastChangeWasUserEdit : 1;
-    unsigned m_isPlaceholderVisible : 1;
-    unsigned m_canShowPlaceholder : 1;
-
-    String m_pointerType { mousePointerEventType() };
-
-    String m_textAsOfLastFormControlChangeEvent;
-
-    unsigned m_cachedSelectionStart;
-    unsigned m_cachedSelectionEnd;
+    unsigned m_lastChangeWasUserEdit : 1 { false };
+    unsigned m_isPlaceholderVisible : 1 { false };
+    unsigned m_canShowPlaceholder : 1 { true };
 
     int m_maxLength { -1 };
     int m_minLength { -1 };
 
+    unsigned m_cachedSelectionStart { 0 };
+    unsigned m_cachedSelectionEnd { 0 };
+
     bool m_hasCachedSelection { false };
+    bool m_hasScheduledSelectionChangeEvent { false };
+
+    String m_pointerType { mousePointerEventType() };
+
+    String m_textAsOfLastFormControlChangeEvent;
 };
 
 WEBCORE_EXPORT HTMLTextFormControlElement* enclosingTextFormControl(const Position&);
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,5 @@


		PASS selectionchange event on each input element fires independently
		PASS selectionchange event on each textarea element fires independently