Bug 1540029 - part 6: Replace HTMLEditRules::GetListItemState() with new stack only class r=m_kato

Differential Revision: https://phabricator.services.mozilla.com/D45788

--HG--
extra : moz-landing-system : lando

editor/libeditor/EditorBase.h

@@ -82,6 +82,7 @@ class InsertNodeTransaction;
 class InsertTextTransaction;
 class JoinNodeTransaction;
 class ListElementSelectionState;
+class ListItemElementSelectionState;
 class PlaceholderTransaction;
 class PresShell;
 class SplitNodeResult;
@@ -2671,6 +2672,7 @@ class EditorBase : public nsIEditor,
   friend class InsertTextTransaction;
   friend class JoinNodeTransaction;
   friend class ListElementSelectionState;
+  friend class ListItemElementSelectionState;
   friend class SplitNodeTransaction;
   friend class TextEditRules;
   friend class TypeInState;

editor/libeditor/HTMLEditRules.cpp

@@ -805,59 +805,62 @@ ListElementSelectionState::ListElementSelectionState(HTMLEditor& aHTMLEditor,
   }
 }
 
-nsresult HTMLEditRules::GetListItemState(bool* aMixed, bool* aLI, bool* aDT,
-                                         bool* aDD) {
-  NS_ENSURE_TRUE(aMixed && aLI && aDT && aDD, NS_ERROR_NULL_POINTER);
-  *aMixed = false;
-  *aLI = false;
-  *aDT = false;
-  *aDD = false;
-  bool bNonList = false;
+ListItemElementSelectionState::ListItemElementSelectionState(
+    HTMLEditor& aHTMLEditor, ErrorResult& aRv) {
+  MOZ_ASSERT(!aRv.Failed());
 
-  if (NS_WARN_IF(!CanHandleEditAction())) {
-    return NS_ERROR_EDITOR_DESTROYED;
+  if (NS_WARN_IF(aHTMLEditor.Destroyed())) {
+    aRv.Throw(NS_ERROR_EDITOR_DESTROYED);
+    return;
   }
 
-  AutoSafeEditorData setData(*this, *mHTMLEditor);
+  // XXX Should we create another constructor which won't create
+  //     AutoEditActionDataSetter?  Or should we create another
+  //     AutoEditActionDataSetter which won't nest edit action?
+  EditorBase::AutoEditActionDataSetter editActionData(aHTMLEditor,
+                                                      EditAction::eNotEditing);
+  if (NS_WARN_IF(!editActionData.CanHandle())) {
+    aRv = EditorBase::ToGenericNSResult(NS_ERROR_EDITOR_DESTROYED);
+    return;
+  }
 
   AutoTArray<OwningNonNull<nsINode>, 64> arrayOfNodes;
-  nsresult rv = HTMLEditorRef().CollectEditTargetNodesInExtendedSelectionRanges(
+  nsresult rv = aHTMLEditor.CollectEditTargetNodesInExtendedSelectionRanges(
       arrayOfNodes, EditSubAction::eCreateOrChangeList,
       HTMLEditor::CollectNonEditableNodes::No);
   if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
+    aRv = EditorBase::ToGenericNSResult(rv);
+    return;
   }
 
   // examine list type for nodes in selection
   for (const auto& node : arrayOfNodes) {
     if (!node->IsElement()) {
-      bNonList = true;
+      mIsOtherElementSelected = true;
     } else if (node->IsAnyOfHTMLElements(nsGkAtoms::ul, nsGkAtoms::ol,
                                          nsGkAtoms::li)) {
-      *aLI = true;
+      mIsLIElementSelected = true;
     } else if (node->IsHTMLElement(nsGkAtoms::dt)) {
-      *aDT = true;
+      mIsDTElementSelected = true;
     } else if (node->IsHTMLElement(nsGkAtoms::dd)) {
-      *aDD = true;
+      mIsDDElementSelected = true;
     } else if (node->IsHTMLElement(nsGkAtoms::dl)) {
-      if (*aDT && *aDD) {
+      if (mIsDTElementSelected && mIsDDElementSelected) {
         continue;
       }
       // need to look inside dl and see which types of items it has
       DefinitionListItemScanner scanner(*node->AsElement());
-      *aDT |= scanner.DTElementFound();
-      *aDD |= scanner.DDElementFound();
+      mIsDTElementSelected |= scanner.DTElementFound();
+      mIsDDElementSelected |= scanner.DDElementFound();
     } else {
-      bNonList = true;
+      mIsOtherElementSelected = true;
+    }
+
+    if (mIsLIElementSelected && mIsDTElementSelected && mIsDDElementSelected &&
+        mIsOtherElementSelected) {
+      break;
     }
   }
-
-  // hokey arithmetic with booleans
-  if (*aDT + *aDD + bNonList > 1) {
-    *aMixed = true;
-  }
-
-  return NS_OK;
 }
 
 nsresult HTMLEditRules::GetAlignment(bool* aMixed,

editor/libeditor/HTMLEditRules.h

@@ -59,8 +59,6 @@ class HTMLEditRules : public TextEditRules {
   virtual nsresult Init(TextEditor* aTextEditor) override;
   virtual nsresult DetachEditor() override;
 
-  MOZ_CAN_RUN_SCRIPT
-  nsresult GetListItemState(bool* aMixed, bool* aLI, bool* aDT, bool* aDD);
   MOZ_CAN_RUN_SCRIPT
   nsresult GetAlignment(bool* aMixed, nsIHTMLEditor::EAlignment* aAlign);
   MOZ_CAN_RUN_SCRIPT

editor/libeditor/HTMLEditor.cpp

@@ -2064,17 +2064,22 @@ HTMLEditor::GetListItemState(bool* aMixed, bool* aLI, bool* aDT, bool* aDD) {
       NS_WARN_IF(!aDD)) {
     return NS_ERROR_INVALID_ARG;
   }
-  if (!mRules) {
+  if (!mInitSucceeded) {
     return NS_ERROR_NOT_INITIALIZED;
   }
 
-  AutoEditActionDataSetter editActionData(*this, EditAction::eNotEditing);
-  if (NS_WARN_IF(!editActionData.CanHandle())) {
-    return NS_ERROR_NOT_INITIALIZED;
+  ErrorResult error;
+  ListItemElementSelectionState state(*this, error);
+  if (NS_WARN_IF(error.Failed())) {
+    return error.StealNSResult();
   }
 
-  RefPtr<HTMLEditRules> htmlRules(mRules->AsHTMLEditRules());
-  return htmlRules->GetListItemState(aMixed, aLI, aDT, aDD);
+  // XXX Why do we ignore `<li>` element selected state?
+  *aMixed = state.IsNotOneTypeDefinitionListItemElementSelected();
+  *aLI = state.IsLIElementSelected();
+  *aDT = state.IsDTElementSelected();
+  *aDD = state.IsDDElementSelected();
+  return NS_OK;
 }
 
 NS_IMETHODIMP

editor/libeditor/HTMLEditor.h

@@ -45,6 +45,7 @@ class AutoSetTemporaryAncestorLimiter;
 class EditActionResult;
 class EmptyEditableFunctor;
 class ListElementSelectionState;
+class ListItemElementSelectionState;
 class MoveNodeResult;
 class ResizerSelectionListener;
 class SplitRangeOffFromNodeResult;
@@ -4366,6 +4367,7 @@ class HTMLEditor final : public TextEditor,
   friend class EmptyEditableFunctor;
   friend class HTMLEditRules;
   friend class ListElementSelectionState;
+  friend class ListItemElementSelectionState;
   friend class SlurpBlobEventListener;
   friend class TextEditor;
   friend class WSRunObject;
@@ -4396,6 +4398,30 @@ class MOZ_STACK_CLASS ListElementSelectionState final {
   bool mIsOtherContentSelected = false;
 };
 
+/**
+ * ListItemElementSelectionState class gets which list item element is selected
+ * right now.
+ */
+class MOZ_STACK_CLASS ListItemElementSelectionState final {
+ public:
+  ListItemElementSelectionState() = delete;
+  ListItemElementSelectionState(HTMLEditor& aHTMLEditor, ErrorResult& aRv);
+
+  bool IsLIElementSelected() const { return mIsLIElementSelected; }
+  bool IsDTElementSelected() const { return mIsDTElementSelected; }
+  bool IsDDElementSelected() const { return mIsDDElementSelected; }
+  bool IsNotOneTypeDefinitionListItemElementSelected() const {
+    return (mIsDTElementSelected + mIsDDElementSelected +
+            mIsOtherElementSelected) > 1;
+  }
+
+ private:
+  bool mIsLIElementSelected = false;
+  bool mIsDTElementSelected = false;
+  bool mIsDDElementSelected = false;
+  bool mIsOtherElementSelected = false;
+};
+
 }  // namespace mozilla
 
 mozilla::HTMLEditor* nsIEditor::AsHTMLEditor() {

editor/libeditor/HTMLEditorCommands.cpp

@@ -303,24 +303,28 @@ nsresult ListItemCommand::GetCurrentState(nsAtom* aTagName,
     return NS_ERROR_INVALID_ARG;
   }
 
-  bool bMixed, bLI, bDT, bDD;
-  nsresult rv = aHTMLEditor->GetListItemState(&bMixed, &bLI, &bDT, &bDD);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  bool inList = false;
-  if (!bMixed) {
-    if (bLI) {
-      inList = aTagName == nsGkAtoms::li;
-    } else if (bDT) {
-      inList = aTagName == nsGkAtoms::dt;
-    } else if (bDD) {
-      inList = aTagName == nsGkAtoms::dd;
-    }
+  ErrorResult error;
+  ListItemElementSelectionState state(*aHTMLEditor, error);
+  if (NS_WARN_IF(error.Failed())) {
+    return error.StealNSResult();
   }
 
-  aParams.SetBool(STATE_ALL, !bMixed && inList);
-  aParams.SetBool(STATE_MIXED, bMixed);
+  if (state.IsNotOneTypeDefinitionListItemElementSelected()) {
+    aParams.SetBool(STATE_ALL, false);
+    aParams.SetBool(STATE_MIXED, true);
+    return NS_OK;
+  }
 
+  nsStaticAtom* selectedListItemTagName = nullptr;
+  if (state.IsLIElementSelected()) {
+    selectedListItemTagName = nsGkAtoms::li;
+  } else if (state.IsDTElementSelected()) {
+    selectedListItemTagName = nsGkAtoms::dt;
+  } else if (state.IsDDElementSelected()) {
+    selectedListItemTagName = nsGkAtoms::dd;
+  }
+  aParams.SetBool(STATE_ALL, aTagName == selectedListItemTagName);
+  aParams.SetBool(STATE_MIXED, false);
   return NS_OK;
 }
 

editor/nsIHTMLEditor.idl

@@ -260,11 +260,13 @@ interface nsIHTMLEditor : nsISupports
    * getListItemState returns what list item type is in the selection.
    * @param aMixed    True if there is more than one type of list item, or
    *                  if there is some list and non-list
+   *                  XXX This ignores `<li>` element selected state.
+   *                      For example, even if `<li>` and `<dt>` are selected,
+   *                      this is set to false.
    * @param aLI       true if "li" list items are selected.
    * @param aDT       true if "dt" list items are selected.
    * @param aDD       true if "dd" list items are selected.
    */
-  [can_run_script]
   void getListItemState(out boolean aMixed, out boolean aLI,
                         out boolean aDT, out boolean aDD);