Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Modal] Marking large trees as aria-hidden causes slowdown in Chrome on Android #21578

Open
2 tasks done
KoenLav opened this issue Jun 26, 2020 · 7 comments
Open
2 tasks done
Labels
accessibility a11y component: modal This is the name of the generic UI component, not the React module! performance

Comments

@KoenLav
Copy link

KoenLav commented Jun 26, 2020

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Currently when opening a Select (or any other Dialog/Modal) aria-hidden=true is added to all sibling elements in the DOM. One of those sibling is obviously the render-target for React, which renders a sizable (but definitely not huge) DOM tree (containing at least a list of 250 elements) in our application.

Setting aria-hidden=true removes the element and its descendants from the accesibility tree, which apparently can be quite expensive on a large-ish DOM on slower devices.

In Chrome on Android on a slower device this causes the entire application to freeze for +/- 5 seconds, whereas this does not seem to occur on Firefox. We have seen a similar problem when hiding the list with display: none (removing it from the DOM and the accessibility tree), which was fixed by using z-index instead of display (this makes sure the elements aren't removed from the tree, but simply not visible).

The problem also occurs when adding aria-hidden=true to the render-target without opening a Select, so this could also be considered as a bug in Chrome (especially since the problem appears to be significantly less (< 1 second) in Firefox on the same device), but I'm reporting it here since maybe there is a better way of hiding (not removing) elements from the accessibility tree?

Expected Behavior 🤔

The application should remain responsive while all siblings of the Dialog/Modal are hidden/removed from the accessibility tree.

Steps to Reproduce 🕹

The application is covered under an NDA, but this should be relatively easy to reproduce on any large-ish DOM on a slower device (or a throttled inspector).

Steps:

  1. Create a large-ish DOM containing a Select;
  2. View it on a slow/throttled device;
  3. Open a select;
  4. Application unresponsive for several seconds.

Context 🔦

I'm hoping we can either find another way to achieve hiding of elements from the accessibility tree, rather than removing them (which would be a better solution for all Material-UI users aside from any browser-specific bugs which may be underlying here) and/or kick this up to Chrome to allow them to improve their underlying code (as Firefox seems less affected).

Your Environment 🌎

Tech Version
Material-UI v4.10.2
React v16.13.1
Browser Chrome on Android, v83.0.4103.106

ps the issue has existed for at least 3 months now, spanning several major versions on Chrome.

@KoenLav KoenLav added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jun 26, 2020
@KoenLav KoenLav changed the title Opening Select causes slowdown in Chrome on Android (caused by aria-hidden) [Select] Opening causes slowdown in Chrome on Android (caused by aria-hidden) Jun 26, 2020
@rikvdlooi
Copy link

To add to this (I work together with KoenLav):

I have tested this on a faster android device, and also there the lag is noticeable (1 - 2 seconds).

We have limited this issue down to be caused by aria-hidden=true, because when we manually add this attribute to the render-target (so it always has this attribute, instead of only when the select is open) there is no lag when opening/closing the select. Also, as Koen already said, when manually adding or removing aria-hidden=true from the render-target, directly after that you can notice a lag for 1 - 2 seconds as well (page is not responsive to clicks/taps during this time).

@oliviertassinari oliviertassinari added the component: select This is the name of the generic UI component, not the React module! label Jun 26, 2020
@oliviertassinari

This comment has been minimized.

@oliviertassinari oliviertassinari marked this as a duplicate of #17001 Jun 26, 2020
@oliviertassinari oliviertassinari added duplicate This issue or pull request already exists performance and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jun 26, 2020
@KoenLav
Copy link
Author

KoenLav commented Jun 26, 2020

@oliviertassinari this is definitely not the same issue.

The list we are rendering is not within the Select component (in fact there are only two options in the Select)., also it probably doesn't need to be a list even, just a large DOM tree outside of the Select component.

Please reopen.

@eps1lon eps1lon removed the duplicate This issue or pull request already exists label Jun 26, 2020
@eps1lon
Copy link
Member

eps1lon commented Jun 26, 2020

Performance implications seem to be unrelated to react but how user agents deal with adding aria-hidden to large trees.

Though I doubt we can do anything here but wait for user agents to improve modals and "inertness" because it is the correct thing to do to "hide" everything but the dialog.

This isssue will be solved for the Select particularly because we will no longer mark the rest of the UI as inert in v5. But the same issue applies to existing components that are modal-like e.g. Dialog.

@eps1lon eps1lon reopened this Jun 26, 2020
@eps1lon eps1lon added component: modal This is the name of the generic UI component, not the React module! and removed component: select This is the name of the generic UI component, not the React module! labels Jun 26, 2020
@eps1lon eps1lon changed the title [Select] Opening causes slowdown in Chrome on Android (caused by aria-hidden) [Modal] Marking large trees as aria-hidden causes slowdown in Chrome on Android Jun 26, 2020
@eps1lon
Copy link
Member

eps1lon commented Jun 26, 2020

The application is covered under an NDA, but this should be relatively easy to reproduce on any large-ish DOM on a slower device (or a throttled inspector).

@KoenLav It would help a lot if you could go ahead and mock such an application in codesandbox.io so that we can debug the issue.

@eps1lon eps1lon added the accessibility a11y label Jun 26, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 26, 2020

@eps1lon Right, sorry, I read the description too quickly. For context, I believe we are waiting for broader support of the aria-modal attribute among screenreaders to move away from the aria-hidden approach.

@paulincai
Copy link

I'll just leave this here too. Meteor is a NodeJS full stack development platform. I personally observe this issue in React and as reported initially in this thread, it only happens on the first load of a modal. https://forums.meteor.com/t/material-ui-v5-dialogs-and-menus-initial-open-lags-freezes/56842. This discussion also contains a repo for reproduction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility a11y component: modal This is the name of the generic UI component, not the React module! performance
Projects
None yet
Development

No branches or pull requests

5 participants