helium/vertical: fix system DnD reattachment for vertical tabs#948
helium/vertical: fix system DnD reattachment for vertical tabs#948alexng353 wants to merge 2 commits intoimputnet:mainfrom
Conversation
wukko
left a comment
wukko
left a comment
There was a problem hiding this comment.
has claude generated the patches directly? that’s not how it should be done, the description alone is definitely hallucinated.
read through this and generate patches properly please: https://github.com/imputnet/helium-macos/blob/main/docs/building.md#basics
p.s. codex is way better at working with helium
Wait, I didn't even see that. Thanks. I don't have codex, sorry. Will fix. Also, this introduces a stupid bug that I thought I fixed but was missed in the patch generation. I'll take a look when I next have time, sorry. |
Two patches: 1. fix-bounds-check-in-get-tab-drag-target: Use the region view's bounds instead of the tab strip view's bounds in GetTabDragTarget. The strip view shrinks when tabs are removed during detachment, causing reattachment to fail. 2. add-system-dnd-support-to-vertical-tab-strip: Add CanDrop, GetDropFormats, OnDragEntered, OnDragUpdated, and OnDragExited to VerticalTabStripRegionView so vertical tabs can be reattached via system DnD on Wayland. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
alexng353
force-pushed
the
alex/fix-vertical-tab-reattachment
branch
from
591ffda to
4273382
Compare
|
Updated the patches to use the correct way of generating them. I have introduced a crash bug
Marking as draft until I resolve this. |
Tab reattachment via system DnD (Wayland) can cause a CHECK failure when the drag controller is destroyed because of a SetCapture() conflict with Wayland's drag grab.
2 participants
Two changes:
Add system DnD support (CanDrop, GetDropFormats, OnDragEntered, OnDragUpdated, OnDragExited) to VerticalTabStripRegionView so that vertical tabs can be reattached via system drag-and-drop on Wayland where move loops are not supported.
Fix bounds check in GetTabDragTarget to use the region view's bounds instead of the tab strip view's bounds. The strip view shrinks when tabs are removed during detachment, causing all reattachment attempts to fail because the cursor falls outside the shrunken bounds.
For your pull request to not get closed without review, please confirm that:
(an approved feature request, or confirmed bug).
otherwise I have marked my PR as draft.
organization if I lied by checking any of these checkboxes.
Tested on (check one or more):
I didn't get an AI to write most of it, but I did have an AI walk me through how the chromium source code works, how the browser handles reattachment, etc. It took me like 2 days to diagnose what was even happening, and this is my first ever contribution to a C++ codebase. Please be very careful looking over the patch provided, as I myself am unaware of the full inner-workings of the tab system. I basically copied code from the horizontal tab system that was missing from the vertical one.
I had to modify another patch, but Claude said that was bad, so instead I had it add another patch in-between the existing patches. Also, I had Claude generate the patches because I mistakenly wrote the code in the chromium build/src directory, which is not exactly where I should have written it.
Thanks!
fixes: #947
fixes: imputnet/helium-linux#201