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.

Sign up for GitHub

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

Jump to bottom

[dialog] Tab crashes with STATUS_ACCESS_VIOLATION when dialog is opened and container query hides sidebar #1224

Closed
Studio384 opened this issue Dec 24, 2024 · 4 comments · Fixed by #1159
Closed

[dialog] Tab crashes with STATUS_ACCESS_VIOLATION when dialog is opened and container query hides sidebar #1224

Studio384 opened this issue Dec 24, 2024 · 4 comments · Fixed by #1159
Labels
component: dialog This is the name of the generic UI component, not the React module!

Comments

@Studio384
Copy link

Bug report

Current behavior

I am facing an issue where whenever I reduce the viewport to the point where a container query triggers to hide a sidebar with some specific classes and content while a Base UI dialog is open, the browser crashes with the error STATUS_ACCESS_VIOLATION in any Chromium based browser.

Recording.2024-12-24.110542.mp4

Expected behavior

The browser does not crash.

Reproducible example

A repository with a minimal reproduction of the issue: https://github.com/Studio384/base-dialog-crash

Base UI version

v1.0.0-alpha.4

Which browser are you using?

Any Chromium-based browser appears to have this issue. Tested on Edge and Chrome 131 on Windows and Qutebrowser 122 on Arch Linux.

Which OS are you using?

Windows 11 24H2, Arch Linux

Which assistive tech are you using (if applicable)?

N/a

Additional context

  • The issue is only reproducible with Base UI's dialog. Material UI and Headless UI do not cause the same issue.
  • Pretty much all of the HTML in the sidebar is required with the exception of the paragraph and the sidebar's background color. Without the SVG, the crash does not happen. Without any of the classes, the crash does not happen. The responsive behavior must come from a container query, a normal responsive class does not reproduce the issue.
  • The dialog is, with exception to the text, a direct copy of the Dialog example provided in the docs.
@github-actions github-actions bot added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Dec 24, 2024
@atomiks
Copy link
Contributor

atomiks commented Dec 24, 2024

It's caused by our (old) scroll lock, we revamped it in #1159 which fixes the issue when I paste it into node_modules. It's not released in any new version yet, we're planning to do a new alpha.5 release early in the new year.

@Studio384
Copy link
Author

That's great to hear.

Purely out of interest, what does it do that results in this error, since it seems to require some very specific circumstances (e.g. why does it only crash when there is an SVG there)?

@atomiks
Copy link
Contributor

atomiks commented Dec 24, 2024 •
edited
Loading

I'm not sure exactly, I just had a feeling it was that. The previous lock adds position: fixed and a negative top offset to the whole <html> tag, which caused problems in Firefox (with sticky elements) and iOS (with scroll containers flashing when the lock was disabled with animations).

I guess the container query on the <body> causes the browser to enter some kind of loop or something because the lock aggressively affects the document layout. It's just not a good solution compared to the new one.

Also for what it's worth, anchored popups might be positioned wrong when the <body> has container-type: inline-size without contain: layout (only in Chrome, currently, and only occurs with positionMethod="fixed" at the moment, which isn't the default): floating-ui/floating-ui#3067. Chrome changed how it worked in v129 and there's this dilemma regarding how to handle it :\

@atomiks atomiks removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Dec 24, 2024
@oliviertassinari oliviertassinari added the component: dialog This is the name of the generic UI component, not the React module! label Dec 24, 2024
@michaldudak michaldudak mentioned this issue Jan 8, 2025
Merged
@michaldudak
Copy link
Member

Closing the issue as it was fixed internally. Expect a release soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component: dialog This is the name of the generic UI component, not the React module!
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[useScrollLock] New implementation atomiks/base-ui
4 participants
@Studio384 @oliviertassinari @michaldudak @atomiks