Pull request overview

This PR addresses intermittent “dead” title-bar dragging for frameless windows on macOS by centralizing drag-to-move behavior in a shared helper that uses a manual top-level widget move path on macOS (and as a fallback when Qt refuses a native system move).

Changes:

• Added FramelessMoveHelper to unify frameless window movement (manual move on macOS; startSystemMove() elsewhere with manual fallback).
• Updated multiple custom title bars / frameless dialogs / pop-outs to use the helper for consistent drag behavior.
• Disabled the macOS startSystemMove() path in the main TitleBar to avoid intermittent refusal/stall behavior.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Thanks for digging into this — the root-cause analysis (consumed press + no fallback when startSystemMove() is silently refused on macOS) matches what the diff fixes. The centralization into FramelessMoveHelper is a nice cleanup; previously every frameless title bar reimplemented the same windowHandle()->startSystemMove() snippet.

A few observations:

Copilot's ContainerTitleBar concern is addressed but worth a second look. The new mouseReleaseEvent does reset m_pressed / OpenHandCursor for the docked path, but only via QTimer::singleShot(0, ...). Why deferred rather than just resetting synchronously on left-button release? src/gui/containers/ContainerTitleBar.cpp:189-205:

QTimer::singleShot(0, this, [this]() {
    if (!m_isFloating && m_pressed
        && !(QGuiApplication::mouseButtons() & Qt::LeftButton)) {
        m_pressed = false;
        setCursor(Qt::OpenHandCursor);
    }
});

The mouseButtons() & LeftButton guard suggests defending against a reentrancy case, but for a plain click-without-drag in docked mode the synchronous reset is straightforward — happy to leave the async version if there's a specific scenario it's protecting against, but a one-line comment explaining what would help.

FramelessMoveHelper.h storing state in QWidget properties is a slightly unusual pattern but works. Worth noting: the helper grabs the mouse on handle (the title bar widget) on macOS, and properties live on that same widget — so if two frameless title bars share a state somehow, they'd collide, but that doesn't happen in practice since each is its own widget.

One subtle behavior change worth confirming: previously in AetherDspDialog, NetworkDiagnosticsDialog, AetherialAudioStrip, and PanadapterApplet, the press handler only consumed the event if windowHandle() existed; otherwise it fell through to the dialog's base handler. The new FramelessMoveHelper::start() always consumes a left-button press on a valid window via the manual path. That's the intended fix on macOS, but on non-macOS if startSystemMove() returns false, the helper now silently swallows the press into a manual move where the old code would have done nothing. That's arguably an improvement (no more dead title bars on WMs that refuse system move), but it does mean these dialogs are slightly more "draggy" on Linux/Windows than before — probably what you want, just flagging.

The TitleBar.cpp change is minimal and clean — #ifndef Q_OS_MAC around the startSystemMove() call inside an already-manual-fallback-capable codepath. m_windowMoveUsesSystem stays false on macOS and the existing manual path runs.

Otherwise nothing blocking. Build is GUI-only and the manual-move math (startPos + globalDelta) is straightforward. Thanks @rfoust!

Claude here, reviewing on Jeremy's behalf.

Verdict

Strong merge. Real macOS-only frameless-drag bug, sound architectural diagnosis, clean centralized helper. Built locally without errors. The one stale-code finding I'd flag (ContainerWidget.cpp:154) appears to be unreachable in the current title-bar state machine — worth a follow-up note, but not a blocker.

Architectural diagnosis

The PR description correctly identifies the root cause: QWindow::startSystemMove() is unreliable on macOS for frameless windows. When Qt/macOS refuses or stalls the native handoff, the mouse press is already consumed and there's no recovery path, so the window appears unresponsive for 5–10 seconds while platform state resolves.

The fix architecture:

• macOS: skip startSystemMove() entirely; always use the manual grabMouse() + window->move() path
• Linux/Windows: try native move first (compositor/WM-managed behavior matters there); fall back to manual if Qt returns false
• One centralized helper so the same logic doesn't drift across the 6+ title-bar implementations

This is the right design call. Manual move on macOS gives up the OS animation polish that native move provides, but in exchange you get a drag that actually works every time. That's a clearly correct tradeoff for frameless windows where the native title bar isn't visible anyway.

Verified

• All 6 frameless title bars migrated to the helper: EditorFramelessTitleBar, FramelessWindowTitleBar, AetherDspDialog, AetherialAudioStrip, NetworkDiagnosticsDialog, PanadapterApplet, StripFinalOutputPanel, ContainerTitleBar — all use FramelessMoveHelper::{start,move,finish} consistently.
• Main TitleBar keeps its custom logic but adds the Q_OS_MAC guard at line 471-474, matching the helper's behavior. PR description correctly explains this scope choice — TitleBar has additional drag-handle hit-testing complexity that would be invasive to migrate.
• CI all green: build, analyze (cpp), check-paths, CodeQL, check-windows. Local build clean (only pre-existing unrelated warning).
• Inline header-only helper — sensible for utility code at this size; each function is < 20 lines.
• State via Qt dynamic properties (_aetherManualWindowMoveActive, _aetherManualWindowMovePressGlobal, _aetherManualWindowMoveStartPos) — gets automatic cleanup when widgets are destroyed. Slightly unusual vs. a member-struct approach, but works. The alternative would be a static QHash<QWidget*, MoveState> which would need destruction wiring.
• mouseButtons() defensive recovery at FramelessMoveHelper.h:76-78 — if the user releases the button without the helper noticing, move() short-circuits to finish(). Good defensive pattern.

One stale-code finding worth noting (not a blocker)

src/gui/containers/ContainerWidget.cpp:154 still calls h->startSystemMove() unconditionally without a return-value check, fallback, or Q_OS_MAC guard:

void ContainerWidget::onTitleBarDragStart(const QPoint&) {
    if (!m_titleBar) return;
    if (m_dockMode == DockMode::Floating) {
        if (auto* w = window())
            if (auto* h = w->windowHandle()) {
                h->startSystemMove();   // ← unguarded; same pattern the PR is removing
                return;
            }
    }
    ...
}

Tracing: ContainerTitleBar::mousePressEvent (the migrated one) does an early return when m_isFloating == true and never sets m_pressed. mouseMoveEvent then short-circuits without emitting dragStartRequested. So ContainerWidget::onTitleBarDragStart is only reachable in the docked (non-floating) case — where m_dockMode == DockMode::Floating is false and the startSystemMove() branch is skipped.

That makes the line currently dead in the floating state but live in the docked state's fall-through (which doesn't enter that branch). I.e., the line at 154 isn't reachable in practice given the current state machine, but it would re-emerge if a future refactor lets m_isFloating and m_dockMode == Floating desync.

Worth a one-line follow-up either (a) removing the dead branch, or (b) routing through FramelessMoveHelper::start() to mirror the rest of the codebase. Not blocking this PR — the bug class this PR addresses doesn't reach that line.

Stale-code audit summary

What I'd want from Jeremy

Per feedback_test_before_commit.md, manual verification on macOS before merge — open a frameless dialog (NetworkDiagnosticsDialog is the easiest), drag it once, release, immediately try to drag again. Pre-PR this would hit the 5–10 second dead zone; post-PR should work continuously. Worth eyeballing the AetherialAudioStrip + popped-out applet panels too since both use the new helper.

Linux/Windows regression check: the startSystemMove() path on those platforms still runs first per the helper's logic, so existing compositor-managed drag behavior is preserved. Worth a quick "drag still feels native on Linux/Windows" check.

Recommendation

Merge after macOS verification. The ContainerWidget.cpp:154 cleanup is a small follow-up worth filing as an issue (or just doing inline).

Thanks @rfoust — the diagnosis (Qt's native-move handoff "stalls or is refused" rather than failing cleanly) is the kind of platform-quirk knowledge that's hard to find without firsthand pain, and the centralized helper means the next title bar someone writes can't accidentally regress this.

73, Jeremy KK7GWY & Claude (AI dev partner)

Approved — centralized helper, all 6 frameless title bars consistent, built clean locally.