Bug 1347809 Set Selection::mCalledByJS to false before moving focus in NotifySelectionListeners() because non-*JS() methods don't set it to false r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 16 Mar 2017 17:15:20 +0900
changeset 500343 6ad562a333aaf781bc94dd680f55be1cb452edea
parent 500342 592ff740f1d7a92b041981240d44f06eba85903f
child 549612 3dc26b98bb35cfe392b8359903ec2e9feb51bdbf
push id49694
push usermasayuki@d-toybox.com
push dateFri, 17 Mar 2017 03:23:12 +0000
reviewerssmaug
bugs1347809
milestone55.0a1
Bug 1347809 Set Selection::mCalledByJS to false before moving focus in NotifySelectionListeners() because non-*JS() methods don't set it to false r?smaug Currently, Selection::NotifySelectionListeners() moves focus before setting mCalledByJS to false. Therefore, that causes internal selection API calls and it may cause moving focus. So, mCalledByJS should be set to false before moving focus in NotifySelectionListeners(). MozReview-Commit-ID: F879bOmhZlv
layout/generic/nsSelection.cpp
--- a/layout/generic/nsSelection.cpp
+++ b/layout/generic/nsSelection.cpp
@@ -6396,21 +6396,27 @@ Selection::NotifySelectionListeners(bool
 }
 
 nsresult
 Selection::NotifySelectionListeners()
 {
   if (!mFrameSelection)
     return NS_OK;//nothing to do
 
+  // Our internal code should not move focus with using this class while
+  // this moves focus nor from selection listeners.
+  AutoRestore<bool> calledByJSRestorer(mCalledByJS);
+  mCalledByJS = false;
+
   // When normal selection is changed by Selection API, we need to move focus
   // if common ancestor of all ranges are in an editing host.  Note that we
   // don't need to move focus *to* the other focusable node because other
   // browsers don't do it either.
-  if (mSelectionType == SelectionType::eNormal && mCalledByJS) {
+  if (mSelectionType == SelectionType::eNormal &&
+      calledByJSRestorer.SavedValue()) {
     nsPIDOMWindowOuter* window = GetWindow();
     nsIDocument* document = GetDocument();
     // If the document is in design mode or doesn't have contenteditable
     // element, we don't need to move focus.
     if (window && document && !document->HasFlag(NODE_IS_EDITABLE) &&
         GetEditor()) {
       RefPtr<Element> newEditingHost = GetCommonEditingHostForAllRanges();
       nsFocusManager* fm = nsFocusManager::GetFocusManager();
@@ -6434,21 +6440,16 @@ Selection::NotifySelectionListeners()
         IgnoredErrorResult err;
         focusedElement->Blur(err);
         NS_WARNING_ASSERTION(!err.Failed(),
                              "Failed to blur focused element");
       }
     }
   }
 
-  // After moving focus, especially in selection listeners, our internal code
-  // should not move focus with using this class.
-  AutoRestore<bool> calledFromExternalRestorer(mCalledByJS);
-  mCalledByJS = false;
-
   if (mFrameSelection->GetBatching()) {
     mFrameSelection->SetDirty();
     return NS_OK;
   }
   nsCOMArray<nsISelectionListener> selectionListeners(mSelectionListeners);
   int32_t cnt = selectionListeners.Count();
   if (cnt != mSelectionListeners.Count()) {
     return NS_ERROR_OUT_OF_MEMORY;  // nsCOMArray is fallible