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(layout): remove focusWindow calls from onWindowCreatedTiling #3233

Merged
merged 1 commit into from
Sep 10, 2023

Conversation

memchr
Copy link
Contributor

@memchr memchr commented Sep 9, 2023

Abstract

Remove calls to focusWindow() from all implementations of onWindowCreatedTiling()

Bug fixed

commit 05047f6 (#2630) introduced a bug where the active workspace incorrectly changes when using silent workspace methods or rules if the target workspace contains an unlocked group.

Current behaviour

When

  • using the moveActiveToWorkspaceSilent dispatcher to move the active window to a target workspace
  • or using the [workspace silent] rules to create a window in the target workspace

and the target workspace contains an unlocked group, the active workspace incorrectly changes to the target workspace instead of remaining in the current workspace as intended (not silent).

Expected behaviour

Active workspace should remain the same when moveActiveToWorkspaceSilent or [workspace silent] are used, regardless the content of target workspace.

Changes

Upon initial static analysis and debugging results, I respectfully suggest that all direct or indirect callers of onWindowCreatedTiling() appear to be calling focusWindow() themselves, or implying that their operand is active within the context. Therefore, the following lines may be redundant or betray the intention of their direct or indirect callers.

g_pCompositor->focusWindow(pWindow);

g_pCompositor->focusWindow(pWindow);

Thus they are removed in this patch.

Please let me know if I have overlooked any significant details or drawn any erroneous assumptions in my conclusions.

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

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

right, it's not the layout's job to manage focus anyways.

@vaxerski vaxerski merged commit a781c15 into hyprwm:main Sep 10, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants