Skip to content

Conversation

@ArchSav
Copy link
Contributor

@ArchSav ArchSav commented Nov 6, 2025

Describe your PR, what does it fix/add?

Implements #11674
When a child window is opened, pin it if the parent is already pinned.

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

Nothing of note.

Is it ready for merging, or does it need work?

Yes it is ready.

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.

tests needed

@ArchSav ArchSav marked this pull request as draft November 6, 2025 23:44
@ArchSav
Copy link
Contributor Author

ArchSav commented Nov 7, 2025

tests needed

Could you elaborate on this a bit,
I am not sure how I would go about making tests for this in hyprtester

@vaxerski
Copy link
Member

vaxerski commented Nov 7, 2025

make a client that listens on stdin and opens a child window when you send something to it, pin it beforehand, check if pinned. See the pointer test client, it does a similar thing for snapping

@ArchSav
Copy link
Contributor Author

ArchSav commented Nov 9, 2025

This should be ready to merge now

@ArchSav ArchSav marked this pull request as ready for review November 9, 2025 10:32
@ArchSav
Copy link
Contributor Author

ArchSav commented Nov 12, 2025

I've implemented those changes
I appreciate the help getting this through :D

@vaxerski
Copy link
Member

no worries, it's my job :)

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.

I think thats it, rest lgtm

vaxerski
vaxerski previously approved these changes Nov 15, 2025
@vaxerski
Copy link
Member

c-f needed

@vaxerski
Copy link
Member

Your test seems to fail

�[32mPassed: �[0mgetFromSocket("/keyword windowrule float, pin, class:child-test-parent"). Got ok
�[31mFailed: �[0mfn(), expected true, got false. Source: hyprtester/src/main.cpp@240.

Rebase on main and fix the windowrule syntax :P

@vaxerski
Copy link
Member

also why the rule even? Just pin the first window via dispatch pin?

@ArchSav
Copy link
Contributor Author

ArchSav commented Dec 17, 2025

I have fixed build problems should be ready to merge

Sorry for the hassle of this

@ArchSav
Copy link
Contributor Author

ArchSav commented Dec 18, 2025

I have fixed the clang format violation

The failed test doesn't show on the local repo and doesn't relate to anything touched by this pr

@ArchSav
Copy link
Contributor Author

ArchSav commented Dec 18, 2025

Merge conflicts fixed

@ArchSav
Copy link
Contributor Author

ArchSav commented Dec 19, 2025

Tests failed at the same place as the 'fix includes' commit. There isn't anything I can do for this

@vaxerski
Copy link
Member

yes thats good that just happens

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.

thanks!

@vaxerski vaxerski merged commit 7bd2073 into hyprwm:main Dec 20, 2025
9 of 10 checks passed
@Aqa-Ib
Copy link
Contributor

Aqa-Ib commented Dec 21, 2025

crash when opening firefox:
hyprlandCrashReport703.txt

@ryand56
Copy link

ryand56 commented Dec 21, 2025

crash when opening firefox: hyprlandCrashReport703.txt

Can reproduce this.

@ItsOhen
Copy link
Contributor

ItsOhen commented Dec 21, 2025

diff --git a/src/protocols/XDGShell.cpp b/src/protocols/XDGShell.cpp
index ebb56342..12698827 100644
--- a/src/protocols/XDGShell.cpp
+++ b/src/protocols/XDGShell.cpp
@@ -257,11 +257,12 @@ CXDGToplevelResource::CXDGToplevelResource(SP<CXdgToplevel> resource_, SP<CXDGSu
 
         m_parent = newp;
 
-        if (m_parent)
+        if (m_parent) {
             m_parent->m_children.emplace_back(m_self);
 
-        if (m_parent->m_window->m_pinned)
-            m_self->m_window->m_pinned = true;
+            if (m_parent->m_window->m_pinned)
+                m_self->m_window->m_pinned = true;
+        }
 
         LOGM(Log::DEBUG, "Toplevel {:x} sets parent to {:x}{}", (uintptr_t)this, (uintptr_t)newp.get(), (oldParent ? std::format(" (was {:x})", (uintptr_t)oldParent.get()) : ""));
     });

@tannerellen
Copy link

Confirmed patch fixes both firefox and thunderbird crash on launch.

@LionHeartP
Copy link

I was crashing when launching Lutris, but the above patch fixed it for me as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants