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

fix: partially revert invalidate focus ring #42126

Merged
merged 1 commit into from May 13, 2024
Merged

Conversation

VerteDinde
Copy link
Member

@VerteDinde VerteDinde commented May 10, 2024

Description of Change

Fixes #41839

We were seeing an inconsistent segfault on Ubuntu and several other Linux distros, and traced it back to this CL:https://chromium-review.googlesource.com/c/chromium/src/+/5344356. It appeats that the current upstream layout invalidation logic doesn't always invalidate a parent and child correctly. A label's bounds could change without being invalidated because its container can change its bounds.

We should handle this layout logic to better handle parent/child invalidation (see #41899, which is in progress) and upstream a fix if possible, but this PR should unblock developers wanting to upgrade to E31 and E30.

cc @ckerr @Kilian

Checklist

Release Notes

Notes: Fixed an inconsistent crash on maximizing window and relayout in Ubuntu.

@VerteDinde VerteDinde requested a review from ckerr May 10, 2024 22:42
@VerteDinde VerteDinde requested a review from a team as a code owner May 10, 2024 22:42
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label May 10, 2024
@VerteDinde VerteDinde added semver/patch backwards-compatible bug fixes target/30-x-y PR should also be added to the "30-x-y" branch. target/31-x-y PR should also be added to the "31-x-y" branch. labels May 10, 2024
Copy link
Member

@ckerr ckerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tldr: 👍 to everything @VerteDinde said in the description.

  • I've confirmed this fixes the issue for me in local builds
  • This is a reasonable way to unblock releases while we figure out the root cause
  • Once we track down the root cause, it would be preferable to fix that and drop this patch if possible

@ckerr ckerr merged commit c56e1df into main May 13, 2024
26 checks passed
@ckerr ckerr deleted the revert-invalidate-focus-ring branch May 13, 2024 16:03
Copy link

release-clerk bot commented May 13, 2024

Release Notes Persisted

Fixed an inconsistent crash on maximizing window and relayout in Ubuntu.

@trop
Copy link
Contributor

trop bot commented May 13, 2024

I have automatically backported this PR to "30-x-y", please check out #42145

@trop trop bot added in-flight/30-x-y and removed target/30-x-y PR should also be added to the "30-x-y" branch. labels May 13, 2024
@trop
Copy link
Contributor

trop bot commented May 13, 2024

I have automatically backported this PR to "31-x-y", please check out #42146

@trop trop bot added in-flight/31-x-y merged/30-x-y PR was merged to the "30-x-y" branch. merged/31-x-y PR was merged to the "31-x-y" branch. and removed target/31-x-y PR should also be added to the "31-x-y" branch. in-flight/30-x-y in-flight/31-x-y labels May 13, 2024
jkleinsc added a commit that referenced this pull request May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged/30-x-y PR was merged to the "30-x-y" branch. merged/31-x-y PR was merged to the "31-x-y" branch. semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: SIGSEGV crash when maximizing window
3 participants