[BREAKING][#666] Default full to attached nodes only; add legacy-full (…

…#764)

Fixes #666

Tabbable has long had legacy behavior that treated detached nodes as
visible. This commit makes the default/full `displayCheck` option
now behave correctly in that it treates detached nodes as hidden.

But since many people may depend on the buggy legacy behavior, a
new 'legacy-full' option is added which preserves the legacy
behavior, for now.

.changeset/dull-wombats-approve.md

@@ -0,0 +1,5 @@
+---
+'tabbable': major
+---
+
+🚨 BREAKING: Default `displayCheck` 'full' option no longer treats detached nodes as visible. Use the new 'legacy-full' option to restore old (incorrect) behavior only if you must. Ideally, make sure tabbable only runs once all nodes of interest have been attached to the document.

README.md

@@ -135,7 +135,7 @@ These options apply to all APIs.
 
 ### displayCheck option
 
-Type: `full` | `non-zero-area` | `none` . Default: `full`.
+Type: `full` | `full-with-hidden` | `non-zero-area` | `none` . Default: `full`.
 
 Configures how to check if an element is displayed.
 
@@ -145,9 +145,14 @@ For this reason Tabbable offers the ability of an alternative way to check if an
 
 The `displayCheck` configuration accepts the following options:
 
-- `full`: (default) Most reliably resembling browser behavior, this option checks that an element is displayed, which requires all of his ancestors are displayed as well (notice that this doesn't exclude `visibility: hidden` or elements with zero size). This option will cause layout reflow, however. If that is a concern, consider the `none` option.
-    - ⚠️ If the container given to `tabbable()` or `focusable()`, or the node given to `isTabbable()` or `isFocusable()`, is not attached to the window's main `document`, the display check will default to __"none"__ (see below for details) because the APIs used to determine a node's display are not supported unless it is attached (i.e. the browser does not calculate its display unless it is attached). This has effectively been tabbable's behavior for a _very_ long time, and you may never have encountered an issue if the nodes with which you used tabbable were always displayed anyway (i.e. the "none" mode assumption was coincidentally correct).
-    - You may encounter the above situation if, for example, you render to a node via React, and this node is [not attached](https://github.com/facebook/react/issues/9117#issuecomment-284228870) to the document (or perhaps, due to timing, it is not _yet_ attached at the time you use tabbable's APIs).
+- `full`: (default) Most reliably resembling browser behavior, this option checks that an element is displayed, which requires it to be attached to the DOM, and for all of his ancestors to be displayed (notice this doesn't exclude `visibility: hidden` or elements with zero size). This option will cause layout reflow, however. If that is a concern, consider the `none` option.
+    - ⚠️ If the container given to `tabbable()` or `focusable()`, or the node given to `isTabbable()` or `isFocusable()`, is not attached to the window's main `document`, the node will be considered hidden and neither tabbable nor focusable. This behavior is new as of `v6.0.0`.
+    - If your code relies on the legacy behavior where detached nodes were considered visible, and you are unable to fix your code to use tabbable once the node is attached, use the `legacy-full` option.
+- `legacy-full`: Same as `full` but restores the __legacy behavior__ of treating detached nodes as visible. This means that if a node is detached, it's then treated as though the display check was set to `none` (see below for details).
+    - ❗️ Since detached nodes are not treated as tabbable/focusable by browsers, using this option is __not recommended__ as it knowingly diverges from browser behavior.
+    - ⚠️ This option may be removed in the future. Tabbable will not maintain it at the expense of new features or if having it makes the code disproportionately more complex. It only exists to make the upgrade path to the correct behavior (i.e. the `full` option) as long and smooth as reasonably possible.
+    - The APIs used to determine a node's display are not supported unless its attached (i.e. the browser does not calculate its display unless it is attached). This has effectively been tabbable's behavior for a _very_ long time (up until the `v6.0.0` release), and you may never have encountered an issue if the nodes with which you used tabbable were always displayed anyway (i.e. the `none` mode assumption was coincidentally correct).
+    - You may encounter the above situation if, for example, you render to a node via React, and this node is [not attached](https://github.com/facebook/react/issues/9117#issuecomment-284228870) to the document (or perhaps, due to timing, it is not _yet_ attached at the time you use tabbable's APIs on it).
 - `non-zero-area`: This option checks display under the assumption that elements that are not displayed have zero area (width AND height equals zero). While not keeping true to browser behavior, this option may enhance accessibility, as zero-size elements with focusable content are considered a strong accessibility anti-pattern.
     - Like the `full` option, this option also causes layout reflow, and should have basically the same performance. Consider the `none` option if reflow is a concern.
     - ⚠️ As with the `full` option, there is a nuance in behavior depending on whether tabbable APIs are executed on attached vs detached nodes using this mode: Attached nodes that are actually displayed will be deemed visible. Detached nodes, _even though displayed_ will always be deemed __hidden__ because detached nodes always have a zero area as the browser does not calculate is dimensions.

index.d.ts

@@ -1,7 +1,7 @@
 type FocusableElement = HTMLElement | SVGElement;
 
 export type CheckOptions = {
-  displayCheck?: 'full' | 'non-zero-area' | 'none';
+  displayCheck?: 'full' | 'legacy-full' | 'non-zero-area' | 'none';
   getShadowRoot?: boolean | ((node: FocusableElement) => ShadowRoot | boolean | undefined);
 };
 

src/index.js

@@ -251,6 +251,44 @@ const isNonTabbableRadio = function (node) {
   return isRadio(node) && !isTabbableRadio(node);
 };
 
+// determines if a node is ultimately attached to the window's document
+const isNodeAttached = function (node) {
+  // The root node is the shadow root if the node is in a shadow DOM; some document otherwise
+  //  (but NOT _the_ document; see second 'If' comment below for more).
+  // If rootNode is shadow root, it'll have a host, which is the element to which the shadow
+  //  is attached, and the one we need to check if it's in the document or not (because the
+  //  shadow, and all nodes it contains, is never considered in the document since shadows
+  //  behave like self-contained DOMs; but if the shadow's HOST, which is part of the document,
+  //  is hidden, or is not in the document itself but is detached, it will affect the shadow's
+  //  visibility, including all the nodes it contains). The host could be any normal node,
+  //  or a custom element (i.e. web component). Either way, that's the one that is considered
+  //  part of the document, not the shadow root, nor any of its children (i.e. the node being
+  //  tested).
+  // To further complicate things, we have to look all the way up until we find a shadow HOST
+  //  that is attached (or find none) because the node might be in nested shadows...
+  // If rootNode is not a shadow root, it won't have a host, and so rootNode should be the
+  //  document (per the docs) and while it's a Document-type object, that document does not
+  //  appear to be the same as the node's `ownerDocument` for some reason, so it's safer
+  //  to ignore the rootNode at this point, and use `node.ownerDocument`. Otherwise,
+  //  using `rootNode.contains(node)` will _always_ be true we'll get false-positives when
+  //  node is actually detached.
+  let nodeRootHost = getRootNode(node).host;
+  let attached = !!(
+    nodeRootHost?.ownerDocument.contains(nodeRootHost) ||
+    node.ownerDocument.contains(node)
+  );
+
+  while (!attached && nodeRootHost) {
+    // since it's not attached and we have a root host, the node MUST be in a nested shadow DOM,
+    //  which means we need to get the host's host and check if that parent host is contained
+    //  in (i.e. attached to) the document
+    nodeRootHost = getRootNode(nodeRootHost).host;
+    attached = !!nodeRootHost?.ownerDocument.contains(nodeRootHost);
+  }
+
+  return attached;
+};
+
 const isZeroArea = function (node) {
   const { width, height } = node.getBoundingClientRect();
   return width === 0 && height === 0;
@@ -271,29 +309,11 @@ const isHidden = function (node, { displayCheck, getShadowRoot }) {
     return true;
   }
 
-  // The root node is the shadow root if the node is in a shadow DOM; some document otherwise
-  //  (but NOT _the_ document; see second 'If' comment below for more).
-  // If rootNode is shadow root, it'll have a host, which is the element to which the shadow
-  //  is attached, and the one we need to check if it's in the document or not (because the
-  //  shadow, and all nodes it contains, is never considered in the document since shadows
-  //  behave like self-contained DOMs; but if the shadow's HOST, which is part of the document,
-  //  is hidden, or is not in the document itself but is detached, it will affect the shadow's
-  //  visibility, including all the nodes it contains). The host could be any normal node,
-  //  or a custom element (i.e. web component). Either way, that's the one that is considered
-  //  part of the document, not the shadow root, nor any of its children (i.e. the node being
-  //  tested).
-  // If rootNode is not a shadow root, it won't have a host, and so rootNode should be the
-  //  document (per the docs) and while it's a Document-type object, that document does not
-  //  appear to be the same as the node's `ownerDocument` for some reason, so it's safer
-  //  to ignore the rootNode at this point, and use `node.ownerDocument`. Otherwise,
-  //  using `rootNode.contains(node)` will _always_ be true we'll get false-positives when
-  //  node is actually detached.
-  const nodeRootHost = getRootNode(node).host;
-  const nodeIsAttached =
-    nodeRootHost?.ownerDocument.contains(nodeRootHost) ||
-    node.ownerDocument.contains(node);
-
-  if (!displayCheck || displayCheck === 'full') {
+  if (
+    !displayCheck ||
+    displayCheck === 'full' ||
+    displayCheck === 'legacy-full'
+  ) {
     if (typeof getShadowRoot === 'function') {
       // figure out if we should consider the node to be in an undisclosed shadow and use the
       //  'non-zero-area' fallback
@@ -333,7 +353,7 @@ const isHidden = function (node, { displayCheck, getShadowRoot }) {
     // NOTE: We must consider case where node is inside a shadow DOM and given directly to
     //  `isTabbable()` or `isFocusable()` -- regardless of `getShadowRoot` option setting.
 
-    if (nodeIsAttached) {
+    if (isNodeAttached(node)) {
       // this works wherever the node is: if there's at least one client rect, it's
       //  somehow displayed; it also covers the CSS 'display: contents' case where the
       //  node itself is hidden in place of its contents; and there's no need to search
@@ -354,6 +374,13 @@ const isHidden = function (node, { displayCheck, getShadowRoot }) {
     //  APIs on nodes in detached containers has actually implicitly used tabbable in what
     //  was later (as of v5.2.0 on Apr 9, 2021) called `displayCheck="none"` mode -- essentially
     //  considering __everything__ to be visible because of the innability to determine styles.
+    //
+    // v6.0.0: As of this major release, the default 'full' option __no longer treats detached
+    //  nodes as visible with the 'none' fallback.__
+    if (displayCheck !== 'legacy-full') {
+      return true; // hidden
+    }
+    // else, fallback to 'none' mode and consider the node visible
   } else if (displayCheck === 'non-zero-area') {
     // NOTE: Even though this tests that the node's client rect is non-zero to determine
     //  whether it's displayed, and that a detached node will __always__ have a zero-area
@@ -363,7 +390,8 @@ const isHidden = function (node, { displayCheck, getShadowRoot }) {
     return isZeroArea(node);
   }
 
-  // visible, as far as we can tell, or per current `displayCheck` mode
+  // visible, as far as we can tell, or per current `displayCheck=none` mode, we assume
+  //  it's visible
   return false;
 };
 

test/e2e/focusable.cy.js

@@ -21,79 +21,84 @@ describe('focusable', () => {
   });
 
   describe('example fixtures', () => {
-    [true, false].forEach((inDocument) => {
-      it(`correctly identifies focusable elements in the "basic" example ${
-        inDocument ? '(container IN doc)' : '(container NOT in doc)'
-      }`, () => {
-        let expectedFocusableIds;
-
-        if (inDocument) {
-          expectedFocusableIds = [
-            'contenteditable-true',
-            'contenteditable-nesting',
-            'contenteditable-negative-tabindex',
-            'contenteditable-NaN-tabindex',
-            'input',
-            'input-readonly',
-            'select',
-            'select-readonly',
-            'href-anchor',
-            'tabindex-hrefless-anchor',
-            'textarea',
-            'textarea-readonly',
-            'button',
-            'tabindex-div',
-            'negative-select',
-            'hiddenParentVisible-button',
-            'displaycontents-child',
-            'audio-control',
-            'audio-control-NaN-tabindex',
-            'video-control',
-            'video-control-NaN-tabindex',
-          ];
-        } else {
-          expectedFocusableIds = [
-            'contenteditable-true',
-            'contenteditable-nesting',
-            'contenteditable-negative-tabindex',
-            'contenteditable-NaN-tabindex',
-            'input',
-            'input-readonly',
-            'select',
-            'select-readonly',
-            'href-anchor',
-            'tabindex-hrefless-anchor',
-            'textarea',
-            'textarea-readonly',
-            'button',
-            'tabindex-div',
-            'negative-select',
-            'displaynone-textarea',
-            'visibilityhidden-button',
-            'hiddenParent-button',
-            'hiddenParentVisible-button',
-            'displaycontents',
-            'displaycontents-child',
-            'displaycontents-child-displaynone',
-            'audio-control',
-            'audio-control-NaN-tabindex',
-            'video-control',
-            'video-control-NaN-tabindex',
-          ];
-        }
+    [undefined, 'full', 'legacy-full'].forEach((displayCheck) => {
+      [true, false].forEach((inDocument) => {
+        it(`correctly identifies focusable elements in the "basic" example ${
+          inDocument ? '(container IN doc' : '(container NOT in doc'
+        }, displayCheck=${displayCheck || '<default>'})`, () => {
+          let expectedFocusableIds;
+
+          if (inDocument) {
+            expectedFocusableIds = [
+              'contenteditable-true',
+              'contenteditable-nesting',
+              'contenteditable-negative-tabindex',
+              'contenteditable-NaN-tabindex',
+              'input',
+              'input-readonly',
+              'select',
+              'select-readonly',
+              'href-anchor',
+              'tabindex-hrefless-anchor',
+              'textarea',
+              'textarea-readonly',
+              'button',
+              'tabindex-div',
+              'negative-select',
+              'hiddenParentVisible-button',
+              'displaycontents-child',
+              'audio-control',
+              'audio-control-NaN-tabindex',
+              'video-control',
+              'video-control-NaN-tabindex',
+            ];
+          } else if (displayCheck === 'legacy-full') {
+            expectedFocusableIds = [
+              'contenteditable-true',
+              'contenteditable-nesting',
+              'contenteditable-negative-tabindex',
+              'contenteditable-NaN-tabindex',
+              'input',
+              'input-readonly',
+              'select',
+              'select-readonly',
+              'href-anchor',
+              'tabindex-hrefless-anchor',
+              'textarea',
+              'textarea-readonly',
+              'button',
+              'tabindex-div',
+              'negative-select',
+              'displaynone-textarea',
+              'visibilityhidden-button',
+              'hiddenParent-button',
+              'hiddenParentVisible-button',
+              'displaycontents',
+              'displaycontents-child',
+              'displaycontents-child-displaynone',
+              'audio-control',
+              'audio-control-NaN-tabindex',
+              'video-control',
+              'video-control-NaN-tabindex',
+            ];
+          } else {
+            // should find nothing because the container will be detached
+            expectedFocusableIds = [];
+          }
 
-        const container = document.createElement('div');
-        container.innerHTML = fixtures.basic;
+          const container = document.createElement('div');
+          container.innerHTML = fixtures.basic;
 
-        if (inDocument) {
-          document.body.append(container);
-        }
+          if (inDocument) {
+            document.body.append(container);
+          }
 
-        const focusableElements = focusable(container);
+          const focusableElements = focusable(container, { displayCheck });
 
-        expect(getIdsFromElementsArray(focusableElements)).to.eql(
-          expectedFocusableIds
-        );
+          expect(getIdsFromElementsArray(focusableElements)).to.eql(
+            expectedFocusableIds
+          );
+        });
       });
     });