Bug 1057858: ensure all frames for '<input type="number">' are created before choosing the nested element to be focused. r=smaug
authorMirko Brodesser <mbrodesser@mozilla.com>
Wed, 17 Apr 2019 07:01:34 +0000
changeset 469812 ee8e96e5d015
parent 469811 e0f7fc19d5cb
child 469813 aa8babb8b1ee
push id112825
push usercbrindusan@mozilla.com
push dateWed, 17 Apr 2019 15:58:37 +0000
treeherdermozilla-inbound@7bd43da7830c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1057858
milestone68.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 1057858: ensure all frames for '<input type="number">' are created before choosing the nested element to be focused. r=smaug - Avoids undesired bluring and focusing of '<input type="number">' and its nested elements. - Add tests for two scenarios where this could occur. Differential Revision: https://phabricator.services.mozilla.com/D27684
dom/base/nsFocusManager.cpp
dom/base/nsFocusManager.h
dom/html/test/forms/test_input_number_focus.html
--- a/dom/base/nsFocusManager.cpp
+++ b/dom/base/nsFocusManager.cpp
@@ -459,17 +459,17 @@ nsFocusManager::SetFocus(Element* aEleme
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsFocusManager::ElementIsFocusable(Element* aElement, uint32_t aFlags,
                                    bool* aIsFocusable) {
   NS_ENSURE_TRUE(aElement, NS_ERROR_INVALID_ARG);
 
-  *aIsFocusable = CheckIfFocusable(aElement, aFlags) != nullptr;
+  *aIsFocusable = FlushAndCheckIfFocusable(aElement, aFlags) != nullptr;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsFocusManager::MoveFocus(mozIDOMWindowProxy* aWindow, Element* aStartElement,
                           uint32_t aType, uint32_t aFlags, Element** aElement) {
   *aElement = nullptr;
@@ -1118,17 +1118,18 @@ void nsFocusManager::ActivateOrDeactivat
   // notification.
   nsContentUtils::CallOnAllRemoteChildren(aWindow, ActivateOrDeactivateChild,
                                           (void*)aActive);
 }
 
 void nsFocusManager::SetFocusInner(Element* aNewContent, int32_t aFlags,
                                    bool aFocusChanged, bool aAdjustWidget) {
   // if the element is not focusable, just return and leave the focus as is
-  RefPtr<Element> elementToFocus = CheckIfFocusable(aNewContent, aFlags);
+  RefPtr<Element> elementToFocus =
+      FlushAndCheckIfFocusable(aNewContent, aFlags);
   if (!elementToFocus) {
     return;
   }
 
   // check if the element to focus is a frame (iframe) containing a child
   // document. Frames are never directly focused; instead focusing a frame
   // means focus what is inside the frame. To do this, the descendant content
   // within the frame is retrieved and that will be focused instead.
@@ -1454,38 +1455,39 @@ bool nsFocusManager::IsNonFocusableRoot(
   // And in userfocusignored context nothing is focusable.
   Document* doc = aContent->GetComposedDoc();
   NS_ASSERTION(doc, "aContent must have current document");
   return aContent == doc->GetRootElement() &&
          (doc->HasFlag(NODE_IS_EDITABLE) || !aContent->IsEditable() ||
           nsContentUtils::IsUserFocusIgnored(aContent));
 }
 
-Element* nsFocusManager::CheckIfFocusable(Element* aElement, uint32_t aFlags) {
+Element* nsFocusManager::FlushAndCheckIfFocusable(Element* aElement,
+                                                  uint32_t aFlags) {
   if (!aElement) return nullptr;
 
-  // this is a special case for some XUL elements or input number, where an
-  // anonymous child is actually focusable and not the element itself.
-  RefPtr<Element> redirectedFocus = GetRedirectedFocus(aElement);
-  if (redirectedFocus) {
-    return CheckIfFocusable(redirectedFocus, aFlags);
-  }
-
   nsCOMPtr<Document> doc = aElement->GetComposedDoc();
   // can't focus elements that are not in documents
   if (!doc) {
     LOGCONTENT("Cannot focus %s because content not in document", aElement)
     return nullptr;
   }
 
   // Make sure that our frames are up to date while ensuring the presshell is
   // also initialized in case we come from a script calling focus() early.
   mEventHandlingNeedsFlush = false;
   doc->FlushPendingNotifications(FlushType::EnsurePresShellInitAndFrames);
 
+  // this is a special case for some XUL elements or input number, where an
+  // anonymous child is actually focusable and not the element itself.
+  RefPtr<Element> redirectedFocus = GetRedirectedFocus(aElement);
+  if (redirectedFocus) {
+    return FlushAndCheckIfFocusable(redirectedFocus, aFlags);
+  }
+
   PresShell* presShell = doc->GetPresShell();
   if (!presShell) {
     return nullptr;
   }
 
   // the root content can always be focused,
   // except in userfocusignored context.
   if (aElement == doc->GetRootElement()) {
@@ -1744,17 +1746,17 @@ void nsFocusManager::Focus(nsPIDOMWindow
   uint32_t focusMethod =
       aFocusChanged ? aFlags & FOCUSMETHODANDRING_MASK
                     : aWindow->GetFocusMethod() | (aFlags & FLAG_SHOWRING);
 
   if (!IsWindowVisible(aWindow)) {
     // if the window isn't visible, for instance because it is a hidden tab,
     // update the current focus and scroll it into view but don't do anything
     // else
-    if (CheckIfFocusable(aElement, aFlags)) {
+    if (FlushAndCheckIfFocusable(aElement, aFlags)) {
       aWindow->SetFocusedElement(aElement, focusMethod);
       if (aFocusChanged) {
         ScrollIntoView(presShell, aElement, aFlags);
       }
     }
     return;
   }
 
@@ -1824,17 +1826,17 @@ void nsFocusManager::Focus(nsPIDOMWindow
       SendFocusOrBlurEvent(eFocus, presShell, doc,
                            aWindow->GetCurrentInnerWindow(),
                            aFlags & FOCUSMETHOD_MASK, aWindowRaised);
     }
   }
 
   // check to ensure that the element is still focusable, and that nothing
   // else was focused during the events above.
-  if (CheckIfFocusable(aElement, aFlags) && mFocusedWindow == aWindow &&
+  if (FlushAndCheckIfFocusable(aElement, aFlags) && mFocusedWindow == aWindow &&
       mFocusedElement == nullptr) {
     mFocusedElement = aElement;
 
     nsIContent* focusedNode = aWindow->GetFocusedElement();
     bool isRefocus = focusedNode && focusedNode->IsEqualNode(aElement);
 
     aWindow->SetFocusedElement(aElement, focusMethod);
 
@@ -3765,17 +3767,18 @@ nsresult nsFocusManager::FocusFirst(Elem
       // If the redirectdocumentfocus attribute is set, redirect the focus to a
       // specific element. This is primarily used to retarget the focus to the
       // urlbar during document navigation.
       nsAutoString retarget;
 
       if (aRootElement->GetAttr(kNameSpaceID_None,
                                 nsGkAtoms::retargetdocumentfocus, retarget)) {
         nsCOMPtr<Element> element = doc->GetElementById(retarget);
-        nsCOMPtr<nsIContent> retargetElement = CheckIfFocusable(element, 0);
+        nsCOMPtr<nsIContent> retargetElement =
+            FlushAndCheckIfFocusable(element, 0);
         if (retargetElement) {
           retargetElement.forget(aNextContent);
           return NS_OK;
         }
       }
     }
 
     nsCOMPtr<nsIDocShell> docShell = doc->GetDocShell();
--- a/dom/base/nsFocusManager.h
+++ b/dom/base/nsFocusManager.h
@@ -253,29 +253,31 @@ class nsFocusManager final : public nsIF
    * I.e., even if aContent is editable root element, this returns true when
    * the document is in designMode.
    *
    * @param aContent must not be null and must be in a document.
    */
   bool IsNonFocusableRoot(nsIContent* aContent);
 
   /**
-   * Checks and returns aContent if it may be focused, another content node if
+   * First flushes the pending notifications to ensure the PresShell and frames
+   * are updated.
+   * Checks and returns aElement if it may be focused, another element node if
    * the focus should be retargeted at another node, or null if the node
    * cannot be focused. aFlags are the flags passed to SetFocus and similar
    * methods.
    *
    * An element is focusable if it is in a document, the document isn't in
    * print preview mode and the element has an nsIFrame where the
-   * CheckIfFocusable method returns true. For <area> elements, there is no
+   * IsFocusable method returns true. For <area> elements, there is no
    * frame, so only the IsFocusable method on the content node must be
    * true.
    */
-  mozilla::dom::Element* CheckIfFocusable(mozilla::dom::Element* aContent,
-                                          uint32_t aFlags);
+  mozilla::dom::Element* FlushAndCheckIfFocusable(
+      mozilla::dom::Element* aElement, uint32_t aFlags);
 
   /**
    * Blurs the currently focused element. Returns false if another element was
    * focused as a result. This would mean that the caller should not proceed
    * with a pending call to Focus. Normally, true would be returned.
    *
    * The currently focused element within aWindowToClear will be cleared.
    * aWindowToClear may be null, which means that no window is cleared. This
--- a/dom/html/test/forms/test_input_number_focus.html
+++ b/dom/html/test/forms/test_input_number_focus.html
@@ -2,40 +2,55 @@
 <html>
 <!--
 https://bugzilla.mozilla.org/show_bug.cgi?id=1268556
 -->
 <head>
   <title>Test focus behaviour for &lt;input type='number'&gt;</title>
   <script src="/tests/SimpleTest/SimpleTest.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <style>
+    #input_test_style_display {
+      display: none;
+    }
+  </style>
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1268556">Mozilla Bug 1268556</a>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1057858">Mozilla Bug 1057858</a>
 <p id="display"></p>
 <div id="content">
-  <input id="input" type="number">
+  <input id="input_test_redirect" type="number">
+  <input id="input_test_style_display" type="number" >
 </div>
 <pre id="test">
 <script type="application/javascript">
 
 /**
  * Test for Bug 1268556.
  * This test checks that when focusing on an input type=number, the focus is
  * redirected to the anonymous text control, but the document.activeElement
  * still returns the <input type=number>.
+ *
+ * Tests for bug 1057858.
+ * Checks that adding an element and immediately focusing it triggers exactly
+ * one "focus" event and no "blur" events. The same for switching
+ * `style.display` from `none` to `block`.
  **/
 SimpleTest.waitForExplicitFinish();
 SimpleTest.waitForFocus(function() {
-  test();
+  test_focus_redirects_to_text_control_but_not_for_activeElement();
+  test_add_element_and_focus_check_one_focus_event();
+  test_style_display_none_change_to_block_check_one_focus_event();
   SimpleTest.finish();
 });
 
-function test() {
-  var number = document.getElementById("input");
+function test_focus_redirects_to_text_control_but_not_for_activeElement() {
+  document.activeElement.blur();
+  var number = document.getElementById("input_test_redirect");
   number.focus();
 
   // The active element returns the input type=number.
   var activeElement = document.activeElement;
   is (activeElement, number, "activeElement should be the number element");
   is (activeElement.localName, "input", "activeElement should be an input element");
   is (activeElement.getAttribute("type"), "number", "activeElement should of type number");
 
@@ -43,12 +58,52 @@ function test() {
   // text control.
   var fm = SpecialPowers.Cc["@mozilla.org/focus-manager;1"]
                         .getService(SpecialPowers.Ci.nsIFocusManager);
   var focusedElement = fm.focusedElement;
   is (focusedElement.localName, "input", "focusedElement should be an input element");
   is (focusedElement.getAttribute("type"), "text", "focusedElement should of type text");
 }
 
+var blurEventCounter = 0;
+var focusEventCounter = 0;
+
+function append_input_element_with_event_listeners_to_dom() {
+  var inputElement = document.createElement("input");
+  inputElement.type = "number";
+  inputElement.addEventListener("blur", function() { ++blurEventCounter; });
+  inputElement.addEventListener("focus", function() { ++focusEventCounter; });
+  var content = document.getElementById("content");
+  content.appendChild(inputElement);
+  return inputElement;
+}
+
+function test_add_element_and_focus_check_one_focus_event() {
+  document.activeElement.blur();
+  var inputElement = append_input_element_with_event_listeners_to_dom();
+
+  blurEventCounter = 0;
+  focusEventCounter = 0;
+  inputElement.focus();
+
+  is(blurEventCounter, 0, "After focus: no blur events observed.");
+  is(focusEventCounter, 1, "After focus: exactly one focus event observed.");
+}
+
+function test_style_display_none_change_to_block_check_one_focus_event() {
+  document.activeElement.blur();
+  var inputElement = document.getElementById("input_test_style_display");
+  inputElement.addEventListener("blur", function() { ++blurEventCounter; });
+  inputElement.addEventListener("focus", function() { ++focusEventCounter; });
+
+  blurEventCounter = 0;
+  focusEventCounter = 0;
+  inputElement.style.display = "block";
+  inputElement.focus();
+
+  is(blurEventCounter, 0, "After focus: no blur events observed.");
+  is(focusEventCounter, 1, "After focus: exactly one focus event observed.");
+}
+
 </script>
 </pre>
 </body>
 </html>