Bug 1746104 - part 7: Make `eQuerySelectedText` event and `ContentEventHandler` work with no selection ranges r=m_kato

Finally, making `WidgetQueryContentEvent::Succeeded` return `true` when there is no selection when its message is `eQuerySelectedText`, and making `ContentEventHandler::OnQuerySelectedText` set the no selection range data even when the queried selection type is "normal. Then, all things which are changed by the previous patches start to work. And this patch adds a simple test for `ContentCache` and update tests of `nsITextInputProcessor`. Differential Revision: https://phabricator.services.mozilla.com/D137432
2022-02-07 22:33:41 +00:00 · 2022-02-07 22:33:41 +00:00 · 69c1675bde
--- a/dom/base/test/chrome/window_nsITextInputProcessor.xhtml
+++ b/dom/base/test/chrome/window_nsITextInputProcessor.xhtml
@ -4155,24 +4155,28 @@ async function runCallbackTests(aForTests)
    if (aNotification.type != "notify-selection-change") {
      return;
    }
-    is(aNotification.offset, aExpected.offset,
-       aDescription + " should cause selection change notification whose offset is " + aExpected.offset);
-    is(aNotification.text, aExpected.text,
-       aDescription + " should cause selection change notification whose text is '" + aExpected.text + "'");
-    is(aNotification.collapsed, aExpected.text.length == 0,
-       aDescription + " should cause selection change notification whose collapsed is " + (aExpected.text.length == 0));
-    is(aNotification.length, aExpected.text.length,
-       aDescription + " should cause selection change notification whose length is " + aExpected.text.length);
-    is(aNotification.reversed, aExpected.reversed || false,
-       aDescription + " should cause selection change notification whose reversed is " + (aExpected.reversed || false));
+    is(aNotification.hasRange, aExpected.hasRange !== false,
+      `${aDescription} should cause selection change notification whose hasRange is ${aExpected.hasRange}`);
+    if (aNotification.hasRange) {
+      is(aNotification.offset, aExpected.offset,
+        `${aDescription} should cause selection change notification whose offset is ${aExpected.offset}`);
+      is(aNotification.text, aExpected.text,
+        `${aDescription} should cause selection change notification whose text is "${aExpected.text}"`);
+      is(aNotification.length, aExpected.text.length,
+        `${aDescription} should cause selection change notification whose length is ${aExpected.text.length}`);
+      is(aNotification.reversed, aExpected.reversed || false,
+        `${aDescription} should cause selection change notification whose reversed is ${aExpected.reversed || false}`);
+    }
+    is(aNotification.collapsed, aExpected.hasRange === false || aExpected.text.length == 0,
+      `${aDescription} should cause selection change notification whose collapsed is ${aExpected.hasRange === false || aExpected.text.length == 0}`);
    is(aNotification.writingMode, aExpected.writingMode || "horizontal-tb",
-       aDescription + " should cause selection change notification whose writingMode is '" + (aExpected.writingMode || "horizontal-tb"));
+      `${aDescription} should cause selection change notification whose writingMode is ${aExpected.writingMode || "horizontal-tb"}`);
    is(aNotification.causedByComposition, aExpected.causedByComposition || false,
-       aDescription + " should cause selection change notification whose causedByComposition is " + (aExpected.causedByComposition || false));
+      `${aDescription} should cause selection change notification whose causedByComposition is ${aExpected.causedByComposition || false}`);
    is(aNotification.causedBySelectionEvent, aExpected.causedBySelectionEvent || false,
-       aDescription + " should cause selection change notification whose causedBySelectionEvent is " + (aExpected.causedBySelectionEvent || false));
+      `${aDescription} should cause selection change notification whose causedBySelectionEvent is ${aExpected.causedBySelectionEvent || false}`);
    is(aNotification.occurredDuringComposition, aExpected.occurredDuringComposition || false,
-       aDescription + " should cause cause selection change notification whose occurredDuringComposition is " + (aExpected.occurredDuringComposition || false));
+      `${aDescription} should cause cause selection change notification whose occurredDuringComposition is ${aExpected.occurredDuringComposition || false}`);
  }

  function checkTextChangeNotification(aNotification, aDescription, aExpected)
@ -4312,6 +4316,18 @@ async function runCallbackTests(aForTests)
  checkSelectionChangeNotification(notifications[0], description + "ArrowRight key press with Shift", { offset: 1, text: "b" });
  dumpUnexpectedNotifications(1);

+  notifications = [];
+  input.editor.selection.removeAllRanges();
+  await waitUntilNotificationsReceived();
+  is(notifications.length, 1,
+    `${description}Removing all selection ranges should cause a selection change notification`);
+  checkSelectionChangeNotification(
+    notifications[0],
+    `${description}Removing all selection ranges in editor`,
+    { hasRange: false }
+  );
+  dumpUnexpectedNotifications(1);
+
  notifications = [];
  let TIP2 = createTIP();
  if (aForTests) {
--- a/dom/events/ContentEventHandler.cpp
+++ b/dom/events/ContentEventHandler.cpp
@ -304,7 +304,8 @@ nsresult ContentEventHandler::InitRootContent(
  return NS_OK;
 }

-nsresult ContentEventHandler::InitCommon(SelectionType aSelectionType,
+nsresult ContentEventHandler::InitCommon(EventMessage aEventMessage,
+                                         SelectionType aSelectionType,
                                         bool aRequireFlush) {
  if (mSelection && mSelection->Type() == aSelectionType) {
    return NS_OK;
@ -351,8 +352,9 @@ nsresult ContentEventHandler::InitCommon(SelectionType aSelectionType,
  }

  // Even if there are no selection ranges, it's usual case if aSelectionType
-  // is a special selection.
-  if (aSelectionType != SelectionType::eNormal) {
+  // is a special selection or we're handling eQuerySelectedText.
+  if (aSelectionType != SelectionType::eNormal ||
+      aEventMessage == eQuerySelectedText) {
    MOZ_ASSERT(!mFirstSelectedRawRange.IsPositioned());
    return NS_OK;
  }
@ -385,7 +387,8 @@ nsresult ContentEventHandler::Init(WidgetQueryContentEvent* aEvent) {
    return NS_ERROR_FAILURE;
  }

-  nsresult rv = InitCommon(selectionType, aEvent->NeedsToFlushLayout());
+  nsresult rv =
+      InitCommon(aEvent->mMessage, selectionType, aEvent->NeedsToFlushLayout());
  NS_ENSURE_SUCCESS(rv, rv);

  // Be aware, WidgetQueryContentEvent::mInput::mOffset should be made absolute
@ -439,7 +442,7 @@ nsresult ContentEventHandler::Init(WidgetQueryContentEvent* aEvent) {
 nsresult ContentEventHandler::Init(WidgetSelectionEvent* aEvent) {
  NS_ASSERTION(aEvent, "aEvent must not be null");

-  nsresult rv = InitCommon();
+  nsresult rv = InitCommon(aEvent->mMessage);
  NS_ENSURE_SUCCESS(rv, rv);

  aEvent->mSucceeded = false;
@ -1317,7 +1320,6 @@ nsresult ContentEventHandler::OnQuerySelectedText(
  MOZ_ASSERT(aEvent->mReply->mOffsetAndData.isNothing());

  if (!mFirstSelectedRawRange.IsPositioned()) {
-    MOZ_ASSERT(aEvent->mInput.mSelectionType != SelectionType::eNormal);
    MOZ_ASSERT(aEvent->mReply->mOffsetAndData.isNothing());
    MOZ_ASSERT_IF(mSelection, !mSelection->RangeCount());
    // This is special case that `mReply` is emplaced, but mOffsetAndData is
--- a/dom/events/ContentEventHandler.h
+++ b/dom/events/ContentEventHandler.h
@ -157,7 +157,8 @@ class MOZ_STACK_CLASS ContentEventHandler {

  nsresult InitBasic(bool aRequireFlush = true);
  MOZ_CAN_RUN_SCRIPT nsresult
-  InitCommon(SelectionType aSelectionType = SelectionType::eNormal,
+  InitCommon(EventMessage aEventMessage,
+             SelectionType aSelectionType = SelectionType::eNormal,
             bool aRequireFlush = true);
  /**
   * InitRootContent() computes the root content of current focused editor.
--- a/dom/events/IMEContentObserver.cpp
+++ b/dom/events/IMEContentObserver.cpp
@ -548,7 +548,7 @@ void IMEContentObserver::OnSelectionChange(Selection& aSelection) {
    return;
  }

-  if (aSelection.RangeCount() && mWidget) {
+  if (mWidget) {
    bool causedByComposition = IsEditorHandlingEventForComposition();
    bool causedBySelectionEvent = TextComposition::IsHandlingSelectionEvent();
    bool duringComposition = IsEditorComposing();
--- a/widget/TextEvents.h
+++ b/widget/TextEvents.h
@ -1043,9 +1043,6 @@ class WidgetQueryContentEvent : public WidgetGUIEvent {
      return false;
    }
    switch (mMessage) {
-      case eQuerySelectedText:
-        return mReply->mOffsetAndData.isSome() ||
-               mInput.mSelectionType != SelectionType::eNormal;
      case eQueryTextContent:
      case eQueryTextRect:
      case eQueryCaretRect:
--- a/widget/tests/browser/browser.ini
+++ b/widget/tests/browser/browser.ini
@ -1,5 +1,7 @@
 [browser_test_clipboardcache.js]
 skip-if = os == 'android' || (os == 'linux' && ccov) || tsan # Bug 1613516, the test consistently timeouts on Linux coverage builds.
+[browser_test_ContentCache.js]
+skip-if = os == 'android'
 [browser_test_swipe_gesture.js]
 run-if = (os == 'mac' || os == 'win')
 support-files =
--- a/widget/tests/browser/browser_test_ContentCache.js
+++ b/widget/tests/browser/browser_test_ContentCache.js
@ -0,0 +1,252 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+add_task(async function() {
+  const TIP = Cc["@mozilla.org/text-input-processor;1"].createInstance(
+    Ci.nsITextInputProcessor
+  );
+  let notifications = [];
+  const observer = (aTIP, aNotification) => {
+    switch (aNotification.type) {
+      case "request-to-commit":
+        aTIP.commitComposition();
+        break;
+      case "request-to-cancel":
+        aTIP.cancelComposition();
+        break;
+      case "notify-end-input-transaction":
+      case "notify-focus":
+      case "notify-blur":
+      case "notify-text-change":
+      case "notify-selection-change":
+        notifications.push(aNotification);
+        break;
+    }
+    return true;
+  };
+
+  function checkNotifications(aExpectedNotifications, aDescription) {
+    for (const expectedNotification of aExpectedNotifications) {
+      const notification = notifications.find(
+        element => element.type == expectedNotification.type
+      );
+      if (expectedNotification.expected) {
+        isnot(
+          notification,
+          undefined,
+          `"${expectedNotification.type}" should be notified ${aDescription}`
+        );
+      } else {
+        is(
+          notification,
+          undefined,
+          `"${expectedNotification.type}" should not be notified ${aDescription}`
+        );
+      }
+    }
+  }
+
+  ok(
+    TIP.beginInputTransaction(window, observer),
+    "nsITextInputProcessor.beingInputTransaction should return true"
+  );
+  ok(
+    TIP.beginInputTransactionForTests(window, observer),
+    "nsITextInputProcessor.beginInputTransactionForTests should return true"
+  );
+
+  await BrowserTestUtils.withNewTab(
+    "https://example.com/browser/toolkit/content/tests/browser/file_empty.html",
+    async function(browser) {
+      ok(browser.isRemoteBrowser, "This test passes only in e10s mode");
+
+      // IMEContentObserver flushes pending IME notifications at next vsync
+      // after something happens.  Therefore, after doing something in content
+      // process, we need to guarantee that IMEContentObserver has a change to
+      // send IME notifications to the main process with calling this function.
+      function waitForSendingIMENotificationsInContent() {
+        return SpecialPowers.spawn(browser, [], async () => {
+          await new Promise(resolve =>
+            content.requestAnimationFrame(() =>
+              content.requestAnimationFrame(resolve)
+            )
+          );
+        });
+      }
+
+      /**
+       * Test when empty editor gets focus
+       */
+      notifications = [];
+      await SpecialPowers.spawn(browser, [], () => {
+        content.document.body.innerHTML = "<div contenteditable><br></div>";
+        const editor = content.document.querySelector("div[contenteditable]");
+        editor.focus();
+      });
+
+      await waitForSendingIMENotificationsInContent();
+
+      (function() {
+        checkNotifications(
+          [
+            { type: "notify-focus", expected: true },
+            { type: "notify-blur", expected: false },
+            { type: "notify-end-input-transaction", expected: false },
+            { type: "notify-text-change", expected: false },
+            { type: "notify-selection-change", expected: false },
+          ],
+          "after empty editor gets focus"
+        );
+        const text = EventUtils.synthesizeQueryTextContent(0, 1000);
+        ok(
+          text?.succeeded,
+          "query text content should succeed after empty editor gets focus"
+        );
+        if (text?.succeeded) {
+          is(
+            text.text.replace(/[\r\n]/g, ""),
+            "",
+            "text should be only line breaks after empty editor gets focus"
+          );
+        }
+        const selection = EventUtils.synthesizeQuerySelectedText();
+        ok(
+          selection?.succeeded,
+          "query selected text should succeed after empty editor gets focus"
+        );
+        if (selection?.succeeded) {
+          ok(
+            !selection.notFound,
+            "query selected text should find a selection range after empty editor gets focus"
+          );
+          if (!selection.notFound) {
+            is(
+              selection.text,
+              "",
+              "selection should be collapsed after empty editor gets focus"
+            );
+          }
+        }
+      })();
+
+      /**
+       * Test when there is non-collapsed selection
+       */
+      notifications = [];
+      await SpecialPowers.spawn(browser, [], () => {
+        const editor = content.document.querySelector("div[contenteditable]");
+        editor.innerHTML = "<p>abc</p><p>def</p>";
+        content
+          .getSelection()
+          .setBaseAndExtent(
+            editor.querySelector("p").firstChild,
+            2,
+            editor.querySelector("p + p").firstChild,
+            1
+          );
+      });
+
+      await waitForSendingIMENotificationsInContent();
+
+      (function() {
+        checkNotifications(
+          [
+            { type: "notify-focus", expected: false },
+            { type: "notify-blur", expected: false },
+            { type: "notify-end-input-transaction", expected: false },
+            { type: "notify-text-change", expected: true },
+            { type: "notify-selection-change", expected: true },
+          ],
+          "after modifying focused editor"
+        );
+        const text = EventUtils.synthesizeQueryTextContent(0, 1000);
+        ok(
+          text?.succeeded,
+          "query text content should succeed after modifying focused editor"
+        );
+        if (text?.succeeded) {
+          is(
+            text.text
+              .trim()
+              .replace(/\r\n/g, "\n")
+              .replace(/\n\n+/g, "\n"),
+            "abc\ndef",
+            "text should include the both paragraph's text after modifying focused editor"
+          );
+        }
+        const selection = EventUtils.synthesizeQuerySelectedText();
+        ok(
+          selection?.succeeded,
+          "query selected text should succeed after modifying focused editor"
+        );
+        if (selection?.succeeded) {
+          ok(
+            !selection.notFound,
+            "query selected text should find a selection range after modifying focused editor"
+          );
+          if (!selection.notFound) {
+            is(
+              selection.text
+                .trim()
+                .replace(/\r\n/g, "\n")
+                .replace(/\n\n+/g, "\n"),
+              "c\nd",
+              "selection should have the selected characters in the both paragraphs after modifying focused editor"
+            );
+          }
+        }
+      })();
+
+      /**
+       * Test when there is no selection ranges
+       */
+      notifications = [];
+      await SpecialPowers.spawn(browser, [], () => {
+        content.getSelection().removeAllRanges();
+      });
+
+      await waitForSendingIMENotificationsInContent();
+
+      (function() {
+        checkNotifications(
+          [
+            { type: "notify-focus", expected: false },
+            { type: "notify-blur", expected: false },
+            { type: "notify-end-input-transaction", expected: false },
+            { type: "notify-text-change", expected: false },
+            { type: "notify-selection-change", expected: true },
+          ],
+          "after removing all selection ranges from the focused editor"
+        );
+        const text = EventUtils.synthesizeQueryTextContent(0, 1000);
+        ok(
+          text?.succeeded,
+          "query text content should succeed after removing all selection ranges from the focused editor"
+        );
+        if (text?.succeeded) {
+          is(
+            text.text
+              .trim()
+              .replace(/\r\n/g, "\n")
+              .replace(/\n\n+/g, "\n"),
+            "abc\ndef",
+            "text should include the both paragraph's text after removing all selection ranges from the focused editor"
+          );
+        }
+        const selection = EventUtils.synthesizeQuerySelectedText();
+        ok(
+          selection?.succeeded,
+          "query selected text should succeed after removing all selection ranges from the focused editor"
+        );
+        if (selection?.succeeded) {
+          ok(
+            selection.notFound,
+            "query selected text should find no selection range after removing all selection ranges from the focused editor"
+          );
+        }
+      })();
+    }
+  );
+});
--- a/widget/tests/moz.build
+++ b/widget/tests/moz.build
@ -7,6 +7,9 @@
 with Files("**"):
    BUG_COMPONENT = ("Core", "Widget")

+with Files("browser/browser_test_ContentCache.html"):
+    BUG_COMPONENT = ("Core", "DOM: UI Events & Focus Handling")
+
 with Files("unit/*macwebapputils*"):
    BUG_COMPONENT = ("Core", "Widget: Cocoa")

--- a/widget/tests/window_composition_text_querycontent.xhtml
+++ b/widget/tests/window_composition_text_querycontent.xhtml
@ -204,7 +204,7 @@ function checkSelection(aExpectedOffset, aExpectedText, aMessage, aID)
    return false;
  }
  if (aExpectedOffset === null) {
-    todo_is(
+    is(
      selectedText.notFound,
      true,
      `${aMessage}: selection should not be found ${aID}`