Test for managing focus is failing

[theinterned]

https://github.com/github/details-dialog-element/runs/5240152268?check_suite_focus=true

This test appears to have been failing since https://github.com/github/details-dialog-element/actions/runs/1766435305 but it looks like older runs may have been failing silently as tests were not running. See for example this run with missing logs https://github.com/github/details-dialog-element/runs/3652343581?check_suite_focus=true

[theinterned]

Discovered this in https://github.com/github/details-dialog-element/pull/72. I spent a bit of time trying to debug but was unsuccessful and bailed.

[theinterned]

With a git bisect @koddsson and I isolated this to https://github.com/github/details-dialog-element/pull/48 it appears that tests were passing at the time so this is likely a change to Chrome since then.

1d61bff9eadeb536120d9dd0cefdb44fbdba4d1a is the first bad commit
commit 1d61bff9eadeb536120d9dd0cefdb44fbdba4d1a
Author: Mu-An Chiou <[email protected]>
Date:   Tue Nov 5 17:03:40 2019 -0500

    Add hidden button in details
    Tested by 'manages focus'

 test/test.js | 1 +
 1 file changed, 1 insertion(+)
bisect run success

Untitled on  HEAD (1d61bff) (BISECTING) [?] is 📦 v3.0.11 via  v16.13.2 took 1m59s 
❯ git show
commit 1d61bff9eadeb536120d9dd0cefdb44fbdba4d1a (HEAD, refs/bisect/bad)
Author: Mu-An Chiou <[email protected]>
Date:   Tue Nov 5 17:03:40 2019 -0500

    Add hidden button in details
    Tested by 'manages focus'

diff --git a/test/test.js b/test/test.js
index 17ba54a..7b24cd3 100644
--- a/test/test.js
+++ b/test/test.js
@@ -29,6 +29,7 @@ describe('details-dialog-element', function() {
 29 ⋮ 29 │             <button data-button>Button</button>
 30 ⋮ 30 │             <button hidden>hidden</button>
 31 ⋮ 31 │             <div hidden><button>hidden</button></div>
    ⋮ 32 │+            <details><button>Button in closed details</button></details>
 32 ⋮ 33 │             <button ${CLOSE_ATTR}>Goodbye</button>
 33 ⋮ 34 │           </details-dialog>
 34 ⋮ 35 │         </details>

[theinterned]

@koddsson and I traced this to the visibile function:

https://github.com/github/details-dialog-element/blob/2aa3a084863368c1e578334742d82c6c10a74c07/src/index.ts#L36-L42

This function fails to score elements that are dependents of a closed details as not visible.

Why? Previously (and in other browsers) elements that were dependents of a closed details had no bounding rect: ie el.offsetWidth and el.offsetHeight both returned 0. However, a recent change in Chromium (https://bugs.chromium.org/p/chromium/issues/detail?id=1276028) means that descendants of a closed details now do have bounding rects.

Solutions

There are a few options:

1. Add code to check if the element is a child of a closed dialog.

we can see this approach in effect in the focus-trap library:

https://github.com/focus-trap/focus-trap/blob/0a5ce1bfd913edc4410aadfc98b1583de9d034cf/docs/demo-bundle.js#L406-L411

var isDirectSummary = matches.call(node, 'details>summary:first-of-type');
var nodeUnderDetails = isDirectSummary ? node.parentElement : node;

if (matches.call(nodeUnderDetails, 'details:not([open]) *')) {
  return true;
}

2. Update the CSS of descendants of a closed dialog

@koddsson asked about our specific use case in the chromium issue and received a useful suggestion to simply set the descendants of a closed details to display: none:

details:not([open]) > :not(summary) {
  display: none;
}