Bug 1624011 - Make constructor of AlignStateAtSelection not assert …

…when there is no selection ranges r=m_kato `AlignStateAtSelection` class is instantiated outside of editor class so that we shouldn't make each user guarantee that there is selection range (fortunately, the putting off cost is really low). And as far as I tested, Blink and WebKit does not throw exception when `Document.qeuryCommand*("justify*")` is called without selection ranges. So, this patch also prevents exception in this situation. Differential Revision: https://phabricator.services.mozilla.com/D68755 --HG-- extra : moz-landing-system : lando
dothq · Apr 1, 2020 · 72cf6dc · 72cf6dc
1 parent 1cc1c59
commit 72cf6dc
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 2 deletions.
diff --git a/editor/libeditor/HTMLEditSubActionHandler.cpp b/editor/libeditor/HTMLEditSubActionHandler.cpp
@@ -883,8 +883,9 @@ AlignStateAtSelection::AlignStateAtSelection(HTMLEditor& aHTMLEditor,
   EditorRawDOMPoint atBodyOrDocumentElement(bodyOrDocumentElement);
 
   nsRange* firstRange = aHTMLEditor.SelectionRefPtr()->GetRangeAt(0);
-  MOZ_ASSERT(firstRange);
-  if (NS_WARN_IF(!firstRange)) {
+  mFoundSelectionRanges = !!firstRange;
+  if (!mFoundSelectionRanges) {
+    NS_WARNING("There was no selection range");
     aRv.Throw(NS_ERROR_FAILURE);
     return;
   }

diff --git a/editor/libeditor/HTMLEditor.h b/editor/libeditor/HTMLEditor.h
@@ -4657,9 +4657,11 @@ class MOZ_STACK_CLASS AlignStateAtSelection final {
   nsIHTMLEditor::EAlignment AlignmentAtSelectionStart() const {
     return mFirstAlign;
   }
+  bool IsSelectionRangesFound() const { return mFoundSelectionRanges; }
 
  private:
   nsIHTMLEditor::EAlignment mFirstAlign = nsIHTMLEditor::eLeft;
+  bool mFoundSelectionRanges = false;
 };
 
 /**

diff --git a/editor/libeditor/HTMLEditorCommands.cpp b/editor/libeditor/HTMLEditorCommands.cpp
@@ -861,6 +861,17 @@ nsresult AlignCommand::GetCurrentState(HTMLEditor* aHTMLEditor,
   ErrorResult error;
   AlignStateAtSelection state(*aHTMLEditor, error);
   if (error.Failed()) {
+    if (!state.IsSelectionRangesFound()) {
+      // If there was no selection ranges, we shouldn't throw exception for
+      // compatibility with the other browsers, but I have no better idea
+      // than returning empty string in this case.  Oddly, Blink/WebKit returns
+      // "true" or "false", but it's different from us and the value does not
+      // make sense.  Additionally, WPT loves our behavior.
+      error.SuppressException();
+      aParams.SetBool(STATE_MIXED, false);
+      aParams.SetCString(STATE_ATTRIBUTE, EmptyCString());
+      return NS_OK;
+    }
     NS_WARNING("AlignStateAtSelection failed");
     return error.StealNSResult();
   }

diff --git a/editor/libeditor/crashtests/1624011.html b/editor/libeditor/crashtests/1624011.html
@@ -0,0 +1,12 @@
+<html>
+<head>
+    <script>
+      window.addEventListener('load', () => {
+        document.designMode = 'on'
+        const selection = document.getSelection()
+        selection.empty()
+        document.queryCommandIndeterm('justifyFull')
+      })
+    </script>
+</head>
+</html>
diff --git a/editor/libeditor/crashtests/crashtests.list b/editor/libeditor/crashtests/crashtests.list
@@ -121,3 +121,4 @@ load 1574544.html
 load 1596516.html
 load 1618906.html
 load 1623913.html
+load 1624011.html