Using display: none instead of visibility: hidden to hide elements in the complex DOM UI.

[lpardosixtosMs]

Filing this issue to follow up on the discussion from the discussion in #308.

@julienw:

I wonder why you decided to use these rules for the closed expandable items:
instead of display: none which is IMO more common. Do you have data about that? When debugging with Firefox' devtools, I can see that the invisible elements were still laid out. I wonder if this might be the cause of the extra slowness in Firefox. If this is common on websites I'll all for keeping it, but otherwise I think this should be changed to display: none so that it's not part of the layout (but still part of the restyle).

@lpardosixtosMs:

This is how the spectrum library is coded. We just used the is-open classname to control whether the item is expanded or not. We can override it if there is consensus that using display: none is better.

Performance-wise we can measure the impact of changing this rule, but that doesn't answer the questions:

• Which one is more realistic?
• Why use one against the other?

Here is a comment from our experts about the latter:

one reason to not use display: none is that, historically, it's not possible to transition/animate from that value. So, if you wanted to have an opening animation where the element grows in height or something like that you might avoid display: none.

[lpardosixtosMs]

I'd like us to rethink the questions above within the context of what the complex DOM workloads intend to accomplish. The complex DOM workloads evaluate the impact of performing seemingly simple small independent actions in a complex web page. In real web pages, most of the current DOM nodes have a display value different from none, so they are part of both: the layout and the re-style. I think that changing to display: none would take us further from the goal of the workload. Here is data for some websites about how many DOM elements have a computed style display value different from none.

Website	% of DOM elements with display value other than none
https://www.amazon.com/	91.93%
https://youtube.com/	76.53%
https://www.google.com/search?q=v8+javascript+engine	94.67%
https://stackoverflow.com/questions/114543/how-can-i-horizontally-center-an-element	90.76%
https://reddit.com/	97.79%
https://www.bestbuy.com/	88.09%
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise	91.17%
https://github.com/WebKit/Speedometer	93.01%
https://www.bing.com/search?q=javascript+v8+engine	85.92%
https://www.apple.com/apple-vision-pro/	95.31%
https://twitter.com/	87.07%
https://www.max.com/	94.66%
https://www.ebay.com/	95.39%
https://cbc.ca/news	92.54%
https://bbc.com/news/world	94.35%
https://nationalgeographic.com/	97.64%
https://timesofindia.indiatimes.com/	96.09%

[julienw]

I don't believe this metric is relevant. A lot of pages use a different value of none for display for very good reasons (inline, inline-block, flex, grid, sticky), but they use display: none a lot too. I believe they use display: none more than visibility: hidden for this. I'm sure visibility: hidden is used in a lot of places too, but that they don't contain as much content (I believe it's used more for accessibility stuff).

[julienw]

I've checked a few examples of trees and things similar to trees, and found diverse results:

• treejs: uses overflow: hidden; height: 0
• fancytree: uses display: none (uses height just for the transition, but switches to display: none after hiding)
• Firefox' JSON view: they don't insert the DOM elements at all when closed
• Slack and Element: they don't insert the DOM elements at all when closed
• codepen's menu: visibility: hidden; max-height: 0; overflow: hidden
• https://ide.kaitai.io/, uses jstree: uses height for transition, then removes the nodes completely
• gmail's labels: remove the DOM nodes
• github's file navigator: remove the DOM nodes
• PHPMyAdmin: uses display: none (uses height just for the transition, but switches to display: none after hiding)

Taking one step back, and I believe this is valid not just for trees but for anything that handle dynamically hiding / showing elements: newer frameworks such as React / Vue make it easy to optionally render elements, therefore I believe that most websites built with these technologies will add/remove the DOM nodes when needed. On the contrary, most websites built with older technologies (but still valid!) that render HTML server-side and minimally act on it with JS and CSS will toggle classes and will use display: none or height + overflow.

I would be happy to have workloads testing all of these cases more. Especially the case of adding / removing a big chunk of DOM nodes.
But in the case that we're considering at the moment, and considering the goal, I believe display: none makes more sense: the displayed nodes are part of restyle+layout, the non-displayed nodes are part of restyle only, and I believe this brings a balanced approach.

Happy to hear more feedback!

[lpardosixtosMs]

As requested in today's meeting, I computed the perf numbers for both versions. Using display: none improves the performance across browsers.

As I stated in my previous comment. I think that switching to display: none is taking too many elements out of the layout, which is not happening in the complex web pages that the complex DOM workload is meant to represent. I'd be happy to hear more feedback.

[julienw]

Thanks for the numbers. It would be good to compare with the numbers for the non-complex versions on the same machine. If you can also run and report them, that would be perfect!

Some quick comments from the numbers above.
It looks like Safari would benefit most from this change, except for Backbone and jQuery, where Firefox benefits most. Chrome benefits a small amount for all tests, Firefox benefits a small amount for the other tests.

For the Firefox performance in Backbone / jQuery, this aligns with the issues we found in #308 (comment). There's a clear performance cliff in Firefox that we should probably chase and fix.

For the Safari performance, this could be related to the performance issues that are still under investigation by the webkit team for some weeks now.

[lpardosixtosMs]

Here are the numbers comparing standalone vs complex.

display: none

visibility: hidden

[bgrins]

We agreed in the meeting to create a blend, because as per #316 (comment) and in-person discussion we think both display: none and visibility: hidden are realistic & widely used ways to achieve this UX.