From 462a5a808ce5c40837e2ada9178e147f8adadccf Mon Sep 17 00:00:00 2001
From: Ben Styles <bstyles@newrelic.com>
Date: Thu, 11 Feb 2021 20:23:30 +0100
Subject: [PATCH 1/2] Don't allow tabbing to elements inside hidden parents

---
 src/__tests__/tab.js | 24 ++++++++++++++++++++++--
 src/tab.js           |  6 +++++-
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/src/__tests__/tab.js b/src/__tests__/tab.js
index de9d0f46..fbd0493d 100644
--- a/src/__tests__/tab.js
+++ b/src/__tests__/tab.js
@@ -17,7 +17,7 @@ test('fires events when tabbing between two elements', () => {
   userEvent.tab()
   expect(getEventSnapshot()).toMatchInlineSnapshot(`
     Events fired on: div
-    
+
     input#a[value=""] - keydown: Tab (9)
     input#a[value=""] - focusout
     input#b[value=""] - focusin
@@ -45,7 +45,7 @@ test('does not change focus if default prevented on keydown', () => {
   userEvent.tab()
   expect(getEventSnapshot()).toMatchInlineSnapshot(`
     Events fired on: div
-    
+
     input#a[value=""] - keydown: Tab (9)
     input#a[value=""] - keyup: Tab (9)
   `)
@@ -418,6 +418,26 @@ test('should not focus disabled elements', () => {
   expect(five).toHaveFocus()
 })
 
+test('should not focus elements inside a hidden parent', () => {
+  setup(`
+    <div>
+      <input data-testid="one" />
+      <div hidden="">
+        <button>click</button>
+      </div>
+      <input data-testid="three" />
+    </div>`)
+
+  const one = document.querySelector('[data-testid="one"]')
+  const three = document.querySelector('[data-testid="three"]')
+
+  userEvent.tab()
+  expect(one).toHaveFocus()
+
+  userEvent.tab()
+  expect(three).toHaveFocus()
+})
+
 test('should keep focus on the document if there are no enabled, focusable elements', () => {
   setup(`<button disabled>no clicky</button>`)
   userEvent.tab()
diff --git a/src/tab.js b/src/tab.js
index 4bca7ff6..5205684b 100644
--- a/src/tab.js
+++ b/src/tab.js
@@ -31,7 +31,11 @@ function tab({shift = false, focusTrap} = {}) {
   const enabledElements = [...focusableElements].filter(
     el =>
       el === previousElement ||
-      (el.getAttribute('tabindex') !== '-1' && !el.disabled),
+      (el.getAttribute('tabindex') !== '-1' &&
+        !el.disabled &&
+        // Hidden elements are taken out of the
+        // document, along with all their children.
+        !el.closest('[hidden]')),
   )
 
   if (enabledElements.length === 0) return

From ee8921d505f2f8a570f1e122b0f7af7a959905b9 Mon Sep 17 00:00:00 2001
From: Philipp Fritsche <ph.fritsche@gmail.com>
Date: Fri, 12 Feb 2021 00:00:35 +0100
Subject: [PATCH 2/2] fix: use isVisible helper to exclude hidden elements

Hidden elements should be excluded from tab order.
Hidden attribute can be overwritten by stylesheets.

There is a bug in jsdom that has to be resolved first.
See https://github.com/jsdom/jsdom/issues/3111
---
 src/__tests__/utils.js | 19 ++++++++++++++++++-
 src/tab.js             |  8 ++++----
 src/utils.js           | 14 ++++++++++++++
 3 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/src/__tests__/utils.js b/src/__tests__/utils.js
index bdd0d0e8..680f5cfa 100644
--- a/src/__tests__/utils.js
+++ b/src/__tests__/utils.js
@@ -1,4 +1,5 @@
-import {isInstanceOfElement} from '../utils'
+import { screen } from '@testing-library/dom'
+import {isInstanceOfElement, isVisible} from '../utils'
 import {setup} from './helpers/utils'
 
 // isInstanceOfElement can be removed once the peerDependency for @testing-library/dom is bumped to a version that includes https://github.com/testing-library/dom-testing-library/pull/885
@@ -71,3 +72,19 @@ describe('check element type per isInstanceOfElement', () => {
     expect(() => isInstanceOfElement(element, 'HTMLSpanElement')).toThrow()
   })
 })
+
+test('check if element is visible', () => {
+  setup(`
+    <input data-testid="visibleInput"/>
+    <input data-testid="hiddenInput" hidden/>
+    <input data-testid="styledHiddenInput" style="display: none">
+    <input data-testid="styledDisplayedInput" hidden style="display: block"/>
+    <div style="display: none"><input data-testid="childInput" /></div>
+  `)
+
+  expect(isVisible(screen.getByTestId('visibleInput'))).toBe(true)
+  expect(isVisible(screen.getByTestId('styledDisplayedInput'))).toBe(true)
+  expect(isVisible(screen.getByTestId('styledHiddenInput'))).toBe(false)
+  expect(isVisible(screen.getByTestId('childInput'))).toBe(false)
+  expect(isVisible(screen.getByTestId('hiddenInput'))).toBe(false)
+})
diff --git a/src/tab.js b/src/tab.js
index 5205684b..975e99ca 100644
--- a/src/tab.js
+++ b/src/tab.js
@@ -1,5 +1,5 @@
 import {fireEvent} from '@testing-library/dom'
-import {getActiveElement, FOCUSABLE_SELECTOR} from './utils'
+import {getActiveElement, FOCUSABLE_SELECTOR, isVisible} from './utils'
 import {focus} from './focus'
 import {blur} from './blur'
 
@@ -33,9 +33,9 @@ function tab({shift = false, focusTrap} = {}) {
       el === previousElement ||
       (el.getAttribute('tabindex') !== '-1' &&
         !el.disabled &&
-        // Hidden elements are taken out of the
-        // document, along with all their children.
-        !el.closest('[hidden]')),
+        // Hidden elements are not tabable
+        isVisible(el)
+      ),
   )
 
   if (enabledElements.length === 0) return
diff --git a/src/utils.js b/src/utils.js
index 96578020..eb47fb99 100644
--- a/src/utils.js
+++ b/src/utils.js
@@ -293,6 +293,19 @@ function isClickable(element) {
   )
 }
 
+function isVisible(element) {
+  const getComputedStyle = getWindowFromNode(element).getComputedStyle
+
+  for(; element && element.ownerDocument; element = element.parentNode) {
+    const display = getComputedStyle(element).display
+    if (display === 'none') {
+      return false
+    }
+  }
+
+  return true
+}
+
 function eventWrapper(cb) {
   let result
   getConfig().eventWrapper(() => {
@@ -367,4 +380,5 @@ export {
   getSelectionRange,
   isContentEditable,
   isInstanceOfElement,
+  isVisible,
 }