Bug 1740989 - Implement focus fixup rule. r=smaug

This implements the proposal in the linked spec issue, and makes
it nightly-only pending resolution + edits.

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

dom/base/Document.cpp

@@ -6490,7 +6490,8 @@ Nullable<WindowProxyHolder> Document::GetDefaultView() const {
   return WindowProxyHolder(win->GetBrowsingContext());
 }
 
-nsIContent* Document::GetUnretargetedFocusedContent() const {
+nsIContent* Document::GetUnretargetedFocusedContent(
+    IncludeChromeOnly aIncludeChromeOnly) const {
   nsCOMPtr<nsPIDOMWindowOuter> window = GetWindow();
   if (!window) {
     return nullptr;
@@ -6506,8 +6507,8 @@ nsIContent* Document::GetUnretargetedFocusedContent() const {
   if (focusedContent->OwnerDoc() != this) {
     return nullptr;
   }
-
+  if (focusedContent->ChromeOnlyAccess() &&
-  if (focusedContent->ChromeOnlyAccess()) {
+      aIncludeChromeOnly == IncludeChromeOnly::No) {
     return focusedContent->FindFirstNonChromeOnlyAccessContent();
   }
   return focusedContent;

dom/base/Document.h

@@ -3357,7 +3357,11 @@ class Document : public nsINode,
                mozilla::ErrorResult& rv);
   Nullable<WindowProxyHolder> GetDefaultView() const;
   Element* GetActiveElement();
-  nsIContent* GetUnretargetedFocusedContent() const;
+  enum class IncludeChromeOnly : bool { No, Yes };
+  // TODO(emilio): Audit callers and remove the default argument, some seem like
+  // they could want the IncludeChromeOnly::Yes version.
+  nsIContent* GetUnretargetedFocusedContent(
+      IncludeChromeOnly = IncludeChromeOnly::No) const;
   bool HasFocus(ErrorResult& rv) const;
   void GetDesignMode(nsAString& aDesignMode);
   void SetDesignMode(const nsAString& aDesignMode,

dom/base/nsContentUtils.cpp

@@ -6654,49 +6654,6 @@ bool nsContentUtils::IsFocusedContent(const nsIContent* aContent) {
   return fm && fm->GetFocusedElement() == aContent;
 }
 
-bool nsContentUtils::IsSubDocumentTabbable(nsIContent* aContent) {
-  Document* doc = aContent->GetComposedDoc();
-  if (!doc) {
-    return false;
-  }
-
-  // If the subdocument lives in another process, the frame is
-  // tabbable.
-  if (EventStateManager::IsRemoteTarget(aContent)) {
-    return true;
-  }
-
-  // XXXbz should this use OwnerDoc() for GetSubDocumentFor?
-  // sXBL/XBL2 issue!
-  Document* subDoc = doc->GetSubDocumentFor(aContent);
-  if (!subDoc) {
-    return false;
-  }
-
-  nsCOMPtr<nsIDocShell> docShell = subDoc->GetDocShell();
-  if (!docShell) {
-    return false;
-  }
-
-  nsCOMPtr<nsIContentViewer> contentViewer;
-  docShell->GetContentViewer(getter_AddRefs(contentViewer));
-  if (!contentViewer) {
-    return false;
-  }
-
-  // If there are 2 viewers for the current docshell, that
-  // means the current document may be a zombie document.
-  // While load and pageshow events are dispatched, zombie viewer is the old,
-  // to be hidden document.
-  if (contentViewer->GetPreviousViewer()) {
-    bool inOnLoad = false;
-    docShell->GetIsExecutingOnLoadHandler(&inOnLoad);
-    return inOnLoad;
-  }
-
-  return true;
-}
-
 bool nsContentUtils::HasScrollgrab(nsIContent* aContent) {
   // If we ever standardize this feature we'll want to hook this up properly
   // again. For now we're removing all the DOM-side code related to it but

dom/base/nsContentUtils.h

@@ -2487,16 +2487,6 @@ class nsContentUtils {
   static void GetAltText(nsAString& text);
   static void GetModifierSeparatorText(nsAString& text);
 
-  /**
-   * Returns if aContent has a tabbable subdocument.
-   * A sub document isn't tabbable when it's a zombie document.
-   *
-   * @param aElement element to test.
-   *
-   * @return Whether the subdocument is tabbable.
-   */
-  static bool IsSubDocumentTabbable(nsIContent* aContent);
-
   /**
    * Returns if aContent has the 'scrollgrab' property.
    * aContent may be null (in this case false is returned).

dom/base/nsFocusManager.cpp

@@ -2096,34 +2096,21 @@ Element* nsFocusManager::FlushAndCheckIfFocusable(Element* aElement,
 
   // If this is an iframe that doesn't have an in-process subdocument, it is
   // either an OOP iframe or an in-process iframe without lazy about:blank
-  // creation having taken place. In the OOP case, treat the frame as
+  // creation having taken place. In the OOP case, iframe is always focusable.
-  // focusable for consistency with Chrome. In the in-process case, create
+  // In the in-process case, create the initial about:blank for in-process
-  // the initial about:blank for in-process BrowsingContexts in order to
+  // BrowsingContexts in order to have the `GetSubDocumentFor` call after this
-  // have the `GetSubDocumentFor` call after this block return something.
+  // block return something.
+  //
+  // TODO(emilio): This block can probably go after bug 543435 lands.
   if (RefPtr<nsFrameLoaderOwner> flo = do_QueryObject(aElement)) {
-    // dom/webauthn/tests/browser/browser_abort_visibility.js fails without
+    if (!aElement->IsXULElement()) {
-    // the exclusion of XUL.
-    if (aElement->NodeInfo()->NamespaceID() != kNameSpaceID_XUL) {
       // Only look at pre-existing browsing contexts. If this function is
       // called during reflow, calling GetBrowsingContext() could cause frame
       // loader initialization at a time when it isn't safe.
       if (BrowsingContext* bc = flo->GetExtantBrowsingContext()) {
         // This call may create a contentViewer-created about:blank.
         // That's intentional, so we can move focus there.
-        Document* subdoc = bc->GetDocument();
+        Unused << bc->GetDocument();
-        if (!subdoc) {
-          return aElement;
-        }
-        nsIPrincipal* framerPrincipal = doc->GetPrincipal();
-        nsIPrincipal* frameePrincipal = subdoc->GetPrincipal();
-        if (framerPrincipal && frameePrincipal &&
-            !framerPrincipal->Equals(frameePrincipal)) {
-          // Assume focusability of different-origin iframes even in the
-          // in-process case for consistency with the OOP case.
-          // This is likely already the case anyway, but in case not,
-          // this makes it explicitly so.
-          return aElement;
-        }
       }
     }
   }

dom/events/test/window_bug1369072.html

@@ -44,11 +44,18 @@ async function runTests()
 
   async function resetScroll()
   {
+    var oldFocus = document.activeElement;
+    var oldFrameFocus = iframe.contentDocument.activeElement;
+
     // Cancel any scroll animation on the target scroll elements to make sure
     // setting scrollTop or scrolLeft works as expected.
+    // cancelScrollAnimation clears focus, so make sure to restore it.
     await cancelScrollAnimation(document.documentElement);
     await cancelScrollAnimation(iframe.contentDocument.documentElement);
 
+    oldFocus.focus();
+    oldFrameFocus.focus();
+
     return new Promise(resolve => {
       var scrollParent = document.documentElement.scrollTop || document.documentElement.scrollLeft;
       var scrollChild = iframe.contentDocument.documentElement.scrollTop || iframe.contentDocument.documentElement.scrollLeft;

dom/html/HTMLObjectElement.cpp

@@ -188,9 +188,8 @@ bool HTMLObjectElement::IsHTMLFocusable(bool aWithMouse, bool* aIsFocusable,
 
   // This method doesn't call nsGenericHTMLFormControlElement intentionally.
   // TODO: It should probably be changed when bug 597242 will be fixed.
-  if (IsEditableRoot() ||
+  if (IsEditableRoot() || Type() == eType_Document ||
-      ((Type() == eType_Document || Type() == eType_FakePlugin) &&
+      Type() == eType_FakePlugin) {
-       nsContentUtils::IsSubDocumentTabbable(this))) {
     if (aTabIndex) {
       *aTabIndex = isFocusable ? attrVal->GetIntegerValue() : 0;
     }

dom/html/nsGenericHTMLFrameElement.cpp

@@ -329,12 +329,7 @@ bool nsGenericHTMLFrameElement::IsHTMLFocusable(bool aWithMouse,
     return true;
   }
 
-  *aIsFocusable = nsContentUtils::IsSubDocumentTabbable(this);
+  *aIsFocusable = true;
-
-  if (!*aIsFocusable && aTabIndex) {
-    *aTabIndex = -1;
-  }
-
   return false;
 }
 

layout/base/PresShell.cpp

@@ -1595,6 +1595,39 @@ void PresShell::AddAuthorSheet(StyleSheet* aSheet) {
   mDocument->ApplicableStylesChanged();
 }
 
+bool PresShell::FixUpFocus() {
+  if (!StaticPrefs::dom_focus_fixup()) {
+    return false;
+  }
+  if (NS_WARN_IF(!mDocument)) {
+    return false;
+  }
+
+  nsIContent* currentFocus = mDocument->GetUnretargetedFocusedContent(
+      Document::IncludeChromeOnly::Yes);
+  if (!currentFocus) {
+    return false;
+  }
+
+  nsIFrame* f = currentFocus->GetPrimaryFrame();
+  if (f && f->IsFocusable()) {
+    return false;
+  }
+
+  if (currentFocus == mDocument->GetBody() ||
+      currentFocus == mDocument->GetRootElement()) {
+    return false;
+  }
+
+  RefPtr fm = nsFocusManager::GetFocusManager();
+  nsCOMPtr<nsPIDOMWindowOuter> window = mDocument->GetWindow();
+  if (NS_WARN_IF(!window)) {
+    return false;
+  }
+  fm->ClearFocus(window);
+  return true;
+}
+
 void PresShell::SelectionWillTakeFocus() {
   if (mSelection) {
     FrameSelectionWillTakeFocus(*mSelection);

layout/base/PresShell.h

@@ -1269,6 +1269,11 @@ class PresShell final : public nsStubDocumentObserver,
   void SelectionWillTakeFocus() override;
   void SelectionWillLoseFocus() override;
 
+  // Implements the "focus fix-up rule". Returns true if the focus moved (in
+  // which case we might need to update layout again).
+  // See https://github.com/whatwg/html/issues/8225
+  MOZ_CAN_RUN_SCRIPT bool FixUpFocus();
+
   /**
    * Set a "resolution" for the document, which if not 1.0 will
    * allocate more or fewer pixels for rescalable content by a factor

layout/base/nsRefreshDriver.cpp

@@ -2580,10 +2580,15 @@ void nsRefreshDriver::Tick(VsyncId aId, TimeStamp aNowTime,
         RefPtr<PresShell> presShell = rawPresShell;
         presShell->mObservingLayoutFlushes = false;
         presShell->mWasLastReflowInterrupted = false;
-        FlushType flushType = HasPendingAnimations(presShell)
+        const auto flushType = HasPendingAnimations(presShell)
                                    ? FlushType::Layout
                                    : FlushType::InterruptibleLayout;
-        presShell->FlushPendingNotifications(ChangesToFlush(flushType, false));
+        const ChangesToFlush ctf(flushType, false);
+        presShell->FlushPendingNotifications(ctf);
+        if (presShell->FixUpFocus()) {
+          presShell->FlushPendingNotifications(ctf);
+        }
+
         // Inform the FontFaceSet that we ticked, so that it can resolve its
         // ready promise if it needs to.
         presShell->NotifyFontFaceSetOnRefresh();

layout/reftests/forms/select/focusring-3-ref.html

@@ -1,36 +0,0 @@
-<!DOCTYPE HTML>
-<html class="reftest-wait"><head>
-    <meta charset="utf-8">
-    <title>Testcase #3 for bug 1253977</title>
-    <style type="text/css">
-
-* { -moz-appearance:none; }
-:focus {
-  border:2px solid black;
-}
-:-moz-focusring {
-  outline: 2px dashed black;
-}
-
-    </style>
-</head>
-<body>
-
-<select><option>1<option>2</select>
-
-<script>
-
-function runTests(){
-  var s = document.querySelector("select");
-  s.focus();
-  document.body.offsetHeight;
-  setTimeout(function(){ document.documentElement.removeAttribute("class"); }, 100);
-}
-
-window.focus();
-window.addEventListener("MozReftestInvalidate", runTests);
-
-</script>
-
-</body>
-</html>

layout/reftests/forms/select/focusring-3.html

@@ -1,40 +0,0 @@
-<!DOCTYPE HTML>
-<html class="reftest-wait"><head>
-    <meta charset="utf-8">
-    <title>Testcase #3 for bug 1253977</title>
-    <style type="text/css">
-
-* { -moz-appearance:none; }
-:focus {
-  border:2px solid black;
-}
-:-moz-focusring {
-  outline: 2px dashed black;
-}
-
-    </style>
-</head>
-<body>
-
-<select onfocus="continueTest1()"><option>1<option>2</select>
-
-<script>
-
-function continueTest1(){
-  var s = document.querySelector("select");
-  setTimeout(function(){ s.style.display = 'none';   }, 2);
-  setTimeout(function(){ s.style.display = 'inline'; document.body.offsetHeight; }, 4);
-  setTimeout(function(){ document.documentElement.removeAttribute("class"); }, 100);
-}
-function runTests(){
-  var s = document.querySelector("select");
-  s.focus();
-}
-
-window.focus();
-window.addEventListener("MozReftestInvalidate", runTests);
-
-</script>
-
-</body>
-</html>

layout/reftests/forms/select/reftest.list

@@ -8,7 +8,6 @@ fuzzy(0-1,0-4) == padding-button-placement.html padding-button-placement-ref.htm
 == 997709-2.html 997709-2-ref.html
 fuzzy(0-4,0-1) needs-focus == focusring-1.html focusring-1-ref.html
 needs-focus == focusring-2.html focusring-2-ref.html
-needs-focus == focusring-3.html focusring-3-ref.html
 == dynamic-text-indent-1.html dynamic-text-indent-1-ref.html
 == dynamic-text-overflow-1.html dynamic-text-overflow-1-ref.html
 == listbox-zero-row-initial.html listbox-zero-row-initial-ref.html

modules/libpref/init/StaticPrefList.yaml

@@ -2265,6 +2265,13 @@
   value: false
   mirror: always
 
+# Controls whether the "focus fixup rule" is enabled. Nightly-only pending
+# resolution + edits in https://github.com/whatwg/html/issues/8225
+- name: dom.focus.fixup
+  type: bool
+  value: @IS_NIGHTLY_BUILD@
+  mirror: always
+
 - name: dom.mouse_capture.enabled
   type: bool
   value: true

testing/web-platform/meta/css/css-contain/container-queries/layout-dependent-focus.html.ini

@@ -1,3 +0,0 @@
-[layout-dependent-focus.html]
-  [Verify that onblur is called on hidden input]
-    expected: FAIL

testing/web-platform/meta/css/selectors/focus-display-none-001.html.ini

@@ -1,7 +0,0 @@
-[focus-display-none-001.html]
-  [Test ':focus' after 'display:none' on input]
-    expected: FAIL
-
-  [Test ':focus' after 'display:none' on input's parent]
-    expected: FAIL
-

testing/web-platform/meta/css/selectors/focus-within-display-none-001.html.ini

@@ -1,7 +0,0 @@
-[focus-within-display-none-001.html]
-  [Test ':focus-within' after 'display:none' on input]
-    expected: FAIL
-
-  [Test ':focus-within' after 'display:none' on input's parent]
-    expected: FAIL
-

testing/web-platform/meta/inert/dynamic-inert-on-focused-element.html.ini

@@ -1,19 +0,0 @@
-[dynamic-inert-on-focused-element.html]
-  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1740989
-  [<input> that gets 'inert' attribute]
-    expected: FAIL
-
-  [<input> whose parent gets 'inert' attribute]
-    expected: FAIL
-
-  [<button> that gets 'inert' attribute]
-    expected: FAIL
-
-  [<div> that gets 'inert' attribute]
-    expected: FAIL
-
-  [<div> whose parent gets 'inert' attribute]
-    expected: FAIL
-
-  [<div> whose grandparent gets 'inert' attribute]
-    expected: FAIL

toolkit/components/prompts/test/test_modal_prompts.html

@@ -1057,8 +1057,7 @@ async function runTests(util) {
     passValue: "",
     checkMsg: "",
     checked: false,
-    focused: null, // nothing focused until after delay fires
+    focused: "button1",
-    defButton: "button0",
     butt0Label: "OK",
     butt1Label: "Cancel",
     butt0Disabled: true,

toolkit/content/xul.css

@@ -88,10 +88,23 @@ button,
 checkbox,
 menulist,
 radiogroup,
+richlistbox,
 tree,
 browser,
 editor,
-iframe {
+iframe,
+label:is(.text-link, [onclick]),
+tab[selected="true"]:not([ignorefocus="true"]) {
+  -moz-user-focus: normal;
+}
+
+/* Avoid losing focus on tabs by keeping them focusable, since some browser
+ * tests rely on this.
+ *
+ * TODO(emilio): Remove this and fix the tests / front-end code:
+ *  * browser/base/content/test/general/browser_tabfocus.js
+ */
+tab:focus {
   -moz-user-focus: normal;
 }
 
@@ -118,10 +131,6 @@ description {
   display: flow-root;
 }
 
-label.text-link, label[onclick] {
-  -moz-user-focus: normal;
-}
-
 label html|span.accesskey {
   text-decoration: underline;
   text-decoration-skip-ink: none;
@@ -477,10 +486,6 @@ tab {
   -moz-box-pack: center;
 }
 
-tab[selected="true"]:not([ignorefocus="true"]) {
-  -moz-user-focus: normal;
-}
-
 tabpanels {
   display: -moz-deck;
 }
@@ -627,7 +632,6 @@ wizardpage {
 /********** Rich Listbox ********/
 
 richlistbox {
-  -moz-user-focus: normal;
   -moz-box-orient: vertical;
 }