Bug 1317718 - Unnecessary QI to nsIContent to check whether text node. r=masayuki
authorMakoto Kato <m_kato@ga2.so-net.ne.jp>
Thu, 17 Nov 2016 11:14:19 +0900
changeset 323246 c97615bbeb94b0ffe5132bece933061ee5eaecf7
parent 323245 dcd436e553ed85cadb6538ca4f4382610a7bcd5f
child 323247 b46f6f9436132594d363812238f2dda7ed640969
push id30971
push usercbook@mozilla.com
push dateFri, 18 Nov 2016 15:51:34 +0000
treeherdermozilla-central@a103e1713a2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmasayuki
bugs1317718
milestone53.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 1317718 - Unnecessary QI to nsIContent to check whether text node. r=masayuki If not text node, QI to nsIContent will be failed. So we should use IsNodeType (Or EditorBase::IsTextNode) without QI for detection of text node. Also, to get content length into text node, we should use TextLength() simply instead of getting content string. It can reduce memory allocation. MozReview-Commit-ID: C8EKxfUBjTP
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/crashtests/1317718.html
editor/libeditor/crashtests/crashtests.list
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -5587,38 +5587,34 @@ HTMLEditRules::GetNodesForOperation(
 
   int32_t rangeCount = aArrayOfRanges.Length();
   if (aTouchContent == TouchContent::yes) {
     // Split text nodes. This is necessary, since GetPromotedPoint() may return a
     // range ending in a text node in case where part of a pre-formatted
     // elements needs to be moved.
     for (int32_t i = 0; i < rangeCount; i++) {
       RefPtr<nsRange> r = aArrayOfRanges[i];
-      nsCOMPtr<nsIContent> endParent = do_QueryInterface(r->GetEndParent());
-      if (!htmlEditor->IsTextNode(endParent)) {
+      nsCOMPtr<nsINode> endParent = r->GetEndParent();
+      if (!endParent->IsNodeOfType(nsINode::eTEXT)) {
         continue;
       }
-      nsCOMPtr<nsIDOMText> textNode = do_QueryInterface(endParent);
-      if (textNode) {
-        int32_t offset = r->EndOffset();
-        nsAutoString tempString;
-        textNode->GetData(tempString);
-
-        if (0 < offset && offset < static_cast<int32_t>(tempString.Length())) {
-          // Split the text node.
-          nsCOMPtr<nsIDOMNode> tempNode;
-          nsresult rv = htmlEditor->SplitNode(endParent->AsDOMNode(), offset,
-                                              getter_AddRefs(tempNode));
-          NS_ENSURE_SUCCESS(rv, rv);
-
-          // Correct the range.
-          // The new end parent becomes the parent node of the text.
-          nsCOMPtr<nsIContent> newParent = endParent->GetParent();
-          r->SetEnd(newParent, newParent->IndexOf(endParent));
-        }
+      int32_t offset = r->EndOffset();
+
+      if (0 < offset &&
+          offset < static_cast<int32_t>(endParent->Length())) {
+        // Split the text node.
+        nsCOMPtr<nsIDOMNode> tempNode;
+        nsresult rv = htmlEditor->SplitNode(endParent->AsDOMNode(), offset,
+                                            getter_AddRefs(tempNode));
+        NS_ENSURE_SUCCESS(rv, rv);
+
+        // Correct the range.
+        // The new end parent becomes the parent node of the text.
+        nsCOMPtr<nsIContent> newParent = endParent->GetParent();
+        r->SetEnd(newParent, newParent->IndexOf(endParent));
       }
     }
   }
 
   // Bust up any inlines that cross our range endpoints, but only if we are
   // allowed to touch content.
 
   if (aTouchContent == TouchContent::yes) {
new file mode 100644
--- /dev/null
+++ b/editor/libeditor/crashtests/1317718.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+<script>
+addEventListener('DOMContentLoaded', function() {
+  let root = document.documentElement;
+  while(root.firstChild) {
+    root.removeChild(root.firstChild);
+  }
+  document.designMode = 'on';
+  document.removeChild(document.documentElement);
+  document.execCommand('justifyleft', false, null);
+});
+</script>
+</html>
--- a/editor/libeditor/crashtests/crashtests.list
+++ b/editor/libeditor/crashtests/crashtests.list
@@ -63,8 +63,9 @@ load 776323.html
 needs-focus load 793866.html
 load 1057677.html
 needs-focus load 1128787.html
 load 1134545.html
 load 1158452.html
 load 1158651.html
 load 1244894.xhtml
 load 1272490.html
+load 1317718.html