Bug 1501663 - part 5: Rewrite the loop in HTMLEditor::GetSelectedElement() with |for| r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 05 Nov 2018 11:07:34 +0000
changeset 444413 a537e68e8002fb646d1555648a5c156cc652731f
parent 444412 7572f410e0c96f3146166cc68678da70ec1fc2aa
child 444414 7a45d67850c42040122363741dd5abc6249765e4
push id109582
push usercsabou@mozilla.com
push dateMon, 05 Nov 2018 16:21:20 +0000
treeherdermozilla-inbound@4115e3cf7ad0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1501663
milestone65.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 1501663 - part 5: Rewrite the loop in HTMLEditor::GetSelectedElement() with |for| r=m_kato HTMLEditor::GetSelectedElement() uses |while| and calls nsIContentIterator::Next() at the last line. Therefore, it cannot use early-continue style. Because nsIContentIterator::Next() is always called, it should be rewrite with |for|. Differential Revision: https://phabricator.services.mozilla.com/D10701
editor/libeditor/HTMLEditor.cpp
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -3002,58 +3002,59 @@ HTMLEditor::GetSelectedElement(const nsA
     }
   }
 
   if (SelectionRefPtr()->IsCollapsed()) {
     return nullptr;
   }
 
   nsCOMPtr<nsIContentIterator> iter = NS_NewContentIterator();
-
-  RefPtr<Element> selectedElement;
-  bool found = false;
   iter->Init(firstRange);
-  // loop through the content iterator for each content node
-  while (!iter->IsDone()) {
+
+  RefPtr<Element> lastElementInRange;
+  for (bool foundElementInRange = false; !iter->IsDone(); iter->Next()) {
     // XXX This is really odd since this means that the result depends on
     //     what is the last node.  If the last node is an element node,
     //     it may be returned even if it does not match with aTagName.
     //     On the other hand, if last node is not an element, i.e., we have
     //     not found proper element node, we return nullptr as this method
     //     name explains.
-    selectedElement = Element::FromNodeOrNull(iter->GetCurrentNode());
-    if (selectedElement) {
-      if (found) {
-        // At least 2 elements are in the range so that return nullptr.
-        return nullptr;
-      }
-
-      if (!aTagName) {
-        found = true;
-      }
-      // The "A" tag is a pain,
-      //  used for both link(href is set) and "Named Anchor"
-      else if ((isLinkTag &&
-                HTMLEditUtils::IsLink(selectedElement)) ||
-               (isNamedAnchorTag &&
-                HTMLEditUtils::IsNamedAnchor(selectedElement))) {
-        found = true;
-      }
-      // All other tag names are handled here.
-      else if (aTagName == selectedElement->NodeInfo()->NameAtom()) {
-        found = true;
-      }
-
-      if (!found) {
-        return nullptr;
-      }
+    lastElementInRange = Element::FromNodeOrNull(iter->GetCurrentNode());
+    if (!lastElementInRange) {
+      continue;
+    }
+
+    if (foundElementInRange) {
+      // At least 2 elements are in the range so that return nullptr.
+      return nullptr;
+    }
+
+    foundElementInRange = true;
+
+    if (!aTagName) {
+      continue;
     }
-    iter->Next();
-  }
-  return selectedElement.forget();
+
+    if (isLinkTag && HTMLEditUtils::IsLink(lastElementInRange)) {
+      continue;
+    }
+
+    if (isNamedAnchorTag && HTMLEditUtils::IsNamedAnchor(lastElementInRange)) {
+      continue;
+    }
+
+    if (aTagName == lastElementInRange->NodeInfo()->NameAtom()) {
+      continue;
+    }
+
+    // First element in the range does not match what the caller is looking
+    // for.
+    return nullptr;
+  }
+  return lastElementInRange.forget();
 }
 
 already_AddRefed<Element>
 HTMLEditor::CreateElementWithDefaults(const nsAtom& aTagName)
 {
   // NOTE: Despite of public method, this can be called for internal use.
 
   // Although this creates an element, but won't change the DOM tree nor