Bug 1548751 - Make TextEditRules::WillSetText() use fast path even if it's for <textarea> element r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 07 May 2019 05:07:14 +0000
changeset 531662 88ce0c2df4d2e3cd324ce99a48e43a55bb695051
parent 531649 7aee5a30dd15cf0e203705808de4fc84cd56393d
child 531663 78ccad22a01cfb6cbf1b47f8f78e1b5d47b7bc6d
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1548751
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1548751 - Make TextEditRules::WillSetText() use fast path even if it's for <textarea> element r=m_kato As far as I've tested, `TextEditor` has the following structure patterns: 1. If it's for a non-empty `<input>` element, it has only one text node. 2. If it's for an empty `<input>` element, it has only bogus node. 3. If it's for a non-empty `<textarea>` element, it has a text node and `moz-<br>` element. Additionally they are followed by `<scrollbar>` and `<resizer>` elements. 4. If it's for an empty `<textarea>` element, it has a `moz-<br>` element followed by `<scrollbar>` and `<resizer>` elements. Additionally, `TextEditRules::WillInsert()` always removes bogus node if there is. So, in the case #2, there is no children. Fortunately, we don't support XUL addons anymore on Firefox. However, in other products like Thunderbird, the tree may be changed as unexpected. Therefore, we still need to keep checking the tree, but we can use the fast path for `<textarea>` element too. Differential Revision: https://phabricator.services.mozilla.com/D30012
editor/libeditor/TextEditRules.cpp
widget/tests/window_composition_text_querycontent.xul
--- a/editor/libeditor/TextEditRules.cpp
+++ b/editor/libeditor/TextEditRules.cpp
@@ -917,34 +917,60 @@ nsresult TextEditRules::WillSetText(bool
   }
 
   nsresult rv = WillInsert(aCancel);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   RefPtr<Element> rootElement = TextEditorRef().GetRoot();
-  uint32_t count = rootElement->GetChildCount();
+  nsIContent* firstChild = rootElement->GetFirstChild();
 
-  // handles only when there is only one node and it's a text node, or empty.
-
-  if (count > 1) {
-    return NS_OK;
+  // We can use this fast path only when:
+  //  - we need to insert a text node.
+  //  - we need to replace content of existing text node.
+  // Additionally, for avoiding odd result, we should check whether we're in
+  // usual condition.
+  if (IsSingleLineEditor()) {
+    // If we're a single line text editor, i.e., <input>, there is only bogus-
+    // <br> element.  Otherwise, there should be only one text node.  But note
+    // that even if there is a bogus node, it's already been removed by
+    // WillInsert().  So, at here, there should be only one text node or no
+    // children.
+    if (firstChild &&
+        (!EditorBase::IsTextNode(firstChild) || firstChild->GetNextSibling())) {
+      return NS_OK;
+    }
+  } else {
+    // If we're a multiline text editor, i.e., <textarea>, there is a moz-<br>
+    // element followed by scrollbar/resizer elements.  Otherwise, a text node
+    // is followed by them.
+    if (!firstChild) {
+      return NS_OK;
+    }
+    if (EditorBase::IsTextNode(firstChild)) {
+      if (!firstChild->GetNextSibling() ||
+          !TextEditUtils::IsMozBR(firstChild->GetNextSibling())) {
+        return NS_OK;
+      }
+    } else if (!TextEditUtils::IsMozBR(firstChild)) {
+      return NS_OK;
+    }
   }
 
   nsAutoString tString(*aString);
 
   if (IsPasswordEditor()) {
     mPasswordText.Assign(tString);
     FillBufWithPWChars(&tString, tString.Length());
   } else if (IsSingleLineEditor()) {
     HandleNewLines(tString);
   }
 
-  if (!count) {
+  if (!firstChild || !EditorBase::IsTextNode(firstChild)) {
     if (tString.IsEmpty()) {
       *aHandled = true;
       return NS_OK;
     }
     RefPtr<Document> doc = TextEditorRef().GetDocument();
     if (NS_WARN_IF(!doc)) {
       return NS_OK;
     }
@@ -963,31 +989,39 @@ nsresult TextEditRules::WillSetText(bool
     }
     *aHandled = true;
 
     ASSERT_PASSWORD_LENGTHS_EQUAL();
 
     return NS_OK;
   }
 
-  nsINode* curNode = rootElement->GetFirstChild();
-  if (NS_WARN_IF(!EditorBase::IsTextNode(curNode))) {
+  // Even if empty text, we don't remove text node and set empty text
+  // for performance
+  RefPtr<Text> textNode = firstChild->GetAsText();
+  if (MOZ_UNLIKELY(NS_WARN_IF(!textNode))) {
     return NS_OK;
   }
-
-  // Even if empty text, we don't remove text node and set empty text
-  // for performance
-  rv = TextEditorRef().SetTextImpl(tString, *curNode->GetAsText());
+  rv = TextEditorRef().SetTextImpl(tString, *textNode);
   if (NS_WARN_IF(!CanHandleEditAction())) {
     return NS_ERROR_EDITOR_DESTROYED;
   }
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
+  // If we replaced non-empty value with empty string, we need to delete the
+  // text node.
+  if (tString.IsEmpty()) {
+    DebugOnly<nsresult> rvIgnored = DidDeleteSelection();
+    MOZ_ASSERT(rvIgnored != NS_ERROR_EDITOR_DESTROYED);
+    NS_WARNING_ASSERTION(NS_SUCCEEDED(rvIgnored),
+                         "DidDeleteSelection() failed");
+  }
+
   *aHandled = true;
 
   ASSERT_PASSWORD_LENGTHS_EQUAL();
 
   return NS_OK;
 }
 
 nsresult TextEditRules::WillSetTextProperty(bool* aCancel, bool* aHandled) {
--- a/widget/tests/window_composition_text_querycontent.xul
+++ b/widget/tests/window_composition_text_querycontent.xul
@@ -7895,18 +7895,17 @@ function* runIMEContentObserverTest()
     dumpUnexpectedNotifications(description, 3);
 
     // []
     description = aDescription + "setting the value property to ''";
     notifications = [];
     aElement.value = "";
     yield waitUntilNotificationsReceived();
     ensureToRemovePrecedingPositionChangeNotification();
-    // XXX Removing invisible <br> or something? The removed length is a line breaker length longer.
-    checkTextChangeNotification(notifications[0], description, { offset: 0, removedLength: getNativeText("ab\ncd\nef\ngh\n").length + kLFLen, addedLength: 0 });
+    checkTextChangeNotification(notifications[0], description, { offset: 0, removedLength: getNativeText("ab\ncd\nef\ngh\n").length, addedLength: 0 });
     checkSelectionChangeNotification(notifications[1], description, { offset: 0, text: "" });
     checkPositionChangeNotification(notifications[2], description);
     ensureToRemovePostPositionChangeNotification(description, 3);
     dumpUnexpectedNotifications(description, 3);
   }
 
   function* testWithHTMLEditor(aDescription, aElement, aDefaultParagraphSeparator)
   {