Skip to content

Commit fd4d20b

Browse files
mrpilot2claude
andcommitted
fix(gui): stop pinning balloon/tooltip popups to Wayland's active window
NotificationBalloon and GotItTooltip were separate Qt::Tool top-level windows positioned via absolute screen coordinates. Wayland gives clients no global coordinate space and ignores/reinterprets such requests for toplevels, so the compositor placed them near whatever window currently had focus instead of the intended anchor (e.g. a balloon anchored to the main window showing inside an unrelated dialog on top of it). Both now stay plain child widgets of their anchor, positioned in its local coordinate space, which needs no protocol-level placement and works identically on X11 and Wayland. Also fixes GotItTooltip sizing too small for wrapped body text: sizeHint() measured height against the layout's pre-resize width instead of the fixed TOOLTIP_WIDTH about to be applied; now uses layout()->totalHeightForWidth() at that width. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 064cb5f commit fd4d20b

5 files changed

Lines changed: 73 additions & 57 deletions

File tree

aide/src/gui/notificationballoonhost.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
#include <QEvent>
66
#include <QGuiApplication>
7+
#include <QPoint>
78
#include <QScreen>
89
#include <QWidget>
910

@@ -101,7 +102,13 @@ NotificationBalloonPlacement NotificationBalloonHost::placement() const
101102

102103
QRect NotificationBalloonHost::anchorRect() const
103104
{
104-
if (m_anchorWidget != nullptr) { return m_anchorWidget->frameGeometry(); }
105+
// Balloons are plain child widgets of the anchor (see
106+
// NotificationBalloon's constructor), so their geometry is positioned
107+
// in the anchor's own local coordinate space, not global screen
108+
// coordinates.
109+
if (m_anchorWidget != nullptr) {
110+
return QRect(QPoint(0, 0), m_anchorWidget->size());
111+
}
105112
if (auto* screen = QGuiApplication::primaryScreen(); screen != nullptr) {
106113
return screen->availableGeometry();
107114
}
@@ -171,6 +178,7 @@ void NotificationBalloonHost::relayout(bool animateNewest)
171178
balloon->slideIn(startPos, targetPos);
172179
} else {
173180
balloon->setGeometry(posX, posY, width, height);
181+
balloon->raise();
174182
}
175183

176184
edgeY =

aide/src/gui/widgets/gotittooltip.cpp

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,14 @@
44

55
#include <QCoreApplication>
66
#include <QEvent>
7-
#include <QGuiApplication>
87
#include <QHBoxLayout>
98
#include <QLabel>
9+
#include <QLayout>
1010
#include <QList>
1111
#include <QPainter>
1212
#include <QPainterPath>
1313
#include <QPolygon>
1414
#include <QPushButton>
15-
#include <QScreen>
1615
#include <QTimer>
1716
#include <QToolButton>
1817
#include <QVBoxLayout>
@@ -174,12 +173,10 @@ GotItTooltip::GotItTooltip(SettingsInterface& settings, const char* id,
174173
, m_linkButton(new QToolButton(this))
175174
, m_gotItButton(new QPushButton(tr("Got it"), this))
176175
{
177-
// No WindowStaysOnTopHint: see NotificationBalloon's constructor for why
178-
// - it floats above every window on the desktop rather than just this
179-
// one, so Alt+Tab to another app left the tooltip stranded on top of
180-
// it. showGotIt() reparents this to the target's window once known,
181-
// which keeps Qt::Tool's transient-window behaviour scoped to it.
182-
setWindowFlags(Qt::FramelessWindowHint | Qt::Tool);
176+
// Stays a plain child widget (see NotificationBalloon's constructor for
177+
// why): showGotIt() reparents this to the target's window once known,
178+
// and a plain child positioned in that window's local coordinate space
179+
// needs no window flags and has no Wayland absolute-positioning issue.
183180
setAttribute(Qt::WA_TranslucentBackground);
184181
setFixedWidth(TOOLTIP_WIDTH);
185182

@@ -325,12 +322,11 @@ void GotItTooltip::showGotIt(QWidget* target, GotItPosition position)
325322
}
326323

327324
m_target = target;
328-
if (auto* window = target->window(); window != nullptr) {
329-
// Transient child of the target's own window (see the constructor's
330-
// comment): keeps this tooltip above only that window, and moved,
331-
// raised, and minimized together with it.
332-
setParent(window, windowFlags());
333-
}
325+
// QWidget::window() is never null (a parentless widget is its own
326+
// window), so this always finds a real anchor. Plain child of the
327+
// target's own window: keeps this tooltip positioned, raised, and
328+
// minimized together with it.
329+
setParent(target->window());
334330
m_position = position;
335331
applyArrowMargins();
336332
enqueue();
@@ -432,10 +428,18 @@ void GotItTooltip::positionNearTarget()
432428
{
433429
if (m_target.isNull()) { return; }
434430

435-
const QRect targetRect(m_target->mapToGlobal(QPoint(0, 0)),
431+
auto* window = m_target->window();
432+
const QRect targetRect(m_target->mapTo(window, QPoint(0, 0)),
436433
m_target->size());
437-
const int width = TOOLTIP_WIDTH;
438-
const int height = sizeHint().height();
434+
const int width = TOOLTIP_WIDTH;
435+
// sizeHint().height() alone under-measures the wrapped body label here:
436+
// it reflects the layout's current width, which may still be the
437+
// widget's pre-resize default rather than the fixed TOOLTIP_WIDTH,
438+
// giving a too-short box for the actual wrapped text. Asking the layout
439+
// for the height at the width we're about to set avoids that.
440+
const int height = layout()->hasHeightForWidth()
441+
? layout()->totalHeightForWidth(width)
442+
: sizeHint().height();
439443
resize(width, height);
440444

441445
int posX = targetRect.left();
@@ -460,16 +464,13 @@ void GotItTooltip::positionNearTarget()
460464
break;
461465
}
462466

463-
if (auto* screen = m_target->screen(); screen != nullptr) {
464-
const auto avail = screen->availableGeometry();
465-
posX = std::clamp(posX, avail.left() + SCREEN_EDGE_MARGIN,
466-
std::max(avail.left() + SCREEN_EDGE_MARGIN,
467-
avail.right() - width - SCREEN_EDGE_MARGIN));
468-
posY =
469-
std::clamp(posY, avail.top() + SCREEN_EDGE_MARGIN,
470-
std::max(avail.top() + SCREEN_EDGE_MARGIN,
471-
avail.bottom() - height - SCREEN_EDGE_MARGIN));
472-
}
467+
const QRect avail = window->rect();
468+
posX = std::clamp(posX, avail.left() + SCREEN_EDGE_MARGIN,
469+
std::max(avail.left() + SCREEN_EDGE_MARGIN,
470+
avail.right() - width - SCREEN_EDGE_MARGIN));
471+
posY = std::clamp(posY, avail.top() + SCREEN_EDGE_MARGIN,
472+
std::max(avail.top() + SCREEN_EDGE_MARGIN,
473+
avail.bottom() - height - SCREEN_EDGE_MARGIN));
473474

474475
move(posX, posY);
475476
}

aide/src/gui/widgets/notificationballoon.cpp

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -117,13 +117,21 @@ NotificationBalloon::NotificationBalloon(NotificationType type,
117117
, m_closeButton(new QToolButton(this))
118118
, m_actionsLayout(new QHBoxLayout)
119119
{
120-
// No WindowStaysOnTopHint: that hint floats above every window on the
121-
// desktop, not just this app's own window, so switching to another app
122-
// (Alt+Tab) left the balloon stranded on top of it. Qt::Tool with a
123-
// parent instead makes this a transient child of the parent's window,
124-
// so the window manager keeps it above only that window and moves,
125-
// raises, and minimizes it together with its owner.
126-
setWindowFlags(Qt::FramelessWindowHint | Qt::Tool);
120+
if (parent == nullptr) {
121+
// No anchor widget available (e.g. headless tests): fall back to
122+
// an independent top-level tool window. No WindowStaysOnTopHint:
123+
// that hint floats above every window on the desktop, not just
124+
// this app's own window.
125+
setWindowFlags(Qt::FramelessWindowHint | Qt::Tool);
126+
}
127+
// Otherwise stay a plain child widget of the anchor. A separate
128+
// top-level (even Qt::Tool, transient-for the anchor) requires
129+
// requesting absolute screen coordinates to position it, which
130+
// Wayland's compositor ignores/reinterprets for toplevels - it placed
131+
// the balloon relative to whatever surface currently had focus instead
132+
// of the anchor. A plain child positioned in the anchor's local
133+
// coordinate space has no such protocol restriction and works
134+
// identically on X11 and Wayland.
127135
setAttribute(Qt::WA_TranslucentBackground);
128136
setFixedWidth(BALLOON_WIDTH);
129137

@@ -256,6 +264,7 @@ void NotificationBalloon::slideIn(const QPoint& fromPos, const QPoint& toPos)
256264
{
257265
move(fromPos);
258266
show();
267+
raise();
259268

260269
m_slideAnimation = new QPropertyAnimation(this, "pos", this);
261270
m_slideAnimation->setDuration(SLIDE_ANIMATION_MS);

aide/src/include/aide/gui/widgets/notificationballoon.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ namespace aide::widgets
4444

4545
/**
4646
* @brief Show the balloon and slide it from @p fromPos to @p toPos
47-
* (both top-left points in global screen coordinates).
47+
* (both top-left points in the parent widget's local coordinate
48+
* space).
4849
*/
4950
void slideIn(const QPoint& fromPos, const QPoint& toPos);
5051

tests/ut/notificationballoonhosttest.cpp

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,12 @@ using aide::widgets::NotificationBalloon;
3131

3232
namespace
3333
{
34-
std::size_t liveBalloonCount()
34+
std::size_t liveBalloonCount(const QWidget& anchor)
3535
{
36-
std::size_t count = 0;
37-
for (auto* widget : QApplication::topLevelWidgets()) {
38-
if (qobject_cast<NotificationBalloon*>(widget) != nullptr) {
39-
++count;
40-
}
41-
}
42-
return count;
36+
return static_cast<std::size_t>(
37+
anchor
38+
.findChildren<NotificationBalloon*>(Qt::FindDirectChildrenOnly)
39+
.size());
4340
}
4441

4542
Notification makeNotification(HierarchicalId groupId)
@@ -77,9 +74,9 @@ TEST_CASE("A NotificationBalloonHost", "[NotificationBalloonHost]")
7774
.displayName = "Build",
7875
.defaultDisplayType = NotificationDisplayType::Balloon});
7976

80-
const auto before = liveBalloonCount();
77+
const auto before = liveBalloonCount(anchor);
8178
manager.post(makeNotification(groupId));
82-
REQUIRE(liveBalloonCount() == before + 1);
79+
REQUIRE(liveBalloonCount(anchor) == before + 1);
8380
}
8481

8582
SECTION("shows a balloon for a StickyBalloon-routed notification")
@@ -89,9 +86,9 @@ TEST_CASE("A NotificationBalloonHost", "[NotificationBalloonHost]")
8986
.displayName = "Build",
9087
.defaultDisplayType = NotificationDisplayType::StickyBalloon});
9188

92-
const auto before = liveBalloonCount();
89+
const auto before = liveBalloonCount(anchor);
9390
manager.post(makeNotification(groupId));
94-
REQUIRE(liveBalloonCount() == before + 1);
91+
REQUIRE(liveBalloonCount(anchor) == before + 1);
9592
}
9693

9794
SECTION("shows no balloon for a None-routed notification")
@@ -101,16 +98,16 @@ TEST_CASE("A NotificationBalloonHost", "[NotificationBalloonHost]")
10198
.displayName = "Build",
10299
.defaultDisplayType = NotificationDisplayType::None});
103100

104-
const auto before = liveBalloonCount();
101+
const auto before = liveBalloonCount(anchor);
105102
manager.post(makeNotification(groupId));
106-
REQUIRE(liveBalloonCount() == before);
103+
REQUIRE(liveBalloonCount(anchor) == before);
107104
}
108105

109106
SECTION("shows no balloon for an unregistered group")
110107
{
111-
const auto before = liveBalloonCount();
108+
const auto before = liveBalloonCount(anchor);
112109
manager.post(makeNotification(groupId));
113-
REQUIRE(liveBalloonCount() == before);
110+
REQUIRE(liveBalloonCount(anchor) == before);
114111
}
115112

116113
SECTION("stacks a balloon per posted notification")
@@ -120,10 +117,10 @@ TEST_CASE("A NotificationBalloonHost", "[NotificationBalloonHost]")
120117
.displayName = "Build",
121118
.defaultDisplayType = NotificationDisplayType::Balloon});
122119

123-
const auto before = liveBalloonCount();
120+
const auto before = liveBalloonCount(anchor);
124121
manager.post(makeNotification(groupId));
125122
manager.post(makeNotification(groupId));
126-
REQUIRE(liveBalloonCount() == before + 2);
123+
REQUIRE(liveBalloonCount(anchor) == before + 2);
127124
}
128125
}
129126

@@ -153,7 +150,7 @@ TEST_CASE("A NotificationBalloonHost's placement setting",
153150
.isValid());
154151

155152
manager.post(makeNotification(groupId));
156-
REQUIRE(liveBalloonCount() > 0);
153+
REQUIRE(liveBalloonCount(anchor) > 0);
157154
}
158155

159156
SECTION("honours an explicit corner setting")
@@ -162,8 +159,8 @@ TEST_CASE("A NotificationBalloonHost's placement setting",
162159
HierarchicalId("notifications")("balloonPlacement"),
163160
static_cast<int>(NotificationBalloonPlacement::TopLeft));
164161

165-
const auto before = liveBalloonCount();
162+
const auto before = liveBalloonCount(anchor);
166163
manager.post(makeNotification(groupId));
167-
REQUIRE(liveBalloonCount() == before + 1);
164+
REQUIRE(liveBalloonCount(anchor) == before + 1);
168165
}
169166
}

0 commit comments

Comments
 (0)