Skip to content

Commit 6900b36

Browse files
committed
fix: prevent false positives in close button detection
Refactor `IsOnCloseButton` to search only within the specific tab node instead of traversing the entire `top_container_view`. Previously, the function would match any `PUSHBUTTON` element in the container, which caused false positives when clicking the New Tab Button or other toolbar buttons. We now introduce `GetTabUnderMouse` to retrieve the tab node under the current mouse position, and update both `HandleCloseButton` and `HandleDoubleClick` to use this.
1 parent b931532 commit 6900b36

3 files changed

Lines changed: 43 additions & 29 deletions

File tree

src/iaccessible.cc

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -445,26 +445,32 @@ int GetTabCount(const NodePtr& top) {
445445
}));
446446
}
447447

448-
// Whether the mouse is on a tab
449-
bool IsOnOneTab(const NodePtr& top, POINT pt) {
448+
// Get the tab node that the mouse is currently on
449+
NodePtr GetTabUnderMouse(const NodePtr& top, POINT pt) {
450450
NodePtr page_tab_pane = FindPageTabPane(top);
451451
if (!page_tab_pane) {
452-
return false;
452+
return nullptr;
453453
}
454454

455-
bool flag = false;
456-
TraversalAccessible(page_tab_pane, [&flag, &pt](const NodePtr& child) {
457-
if (GetAccessibleRole(child) != ROLE_SYSTEM_PAGETAB) {
458-
return false;
459-
}
460-
GetAccessibleSize(child, [&flag, &pt](RECT rect) {
461-
if (PtInRect(&rect, pt)) {
462-
flag = true;
463-
}
464-
});
465-
return flag;
466-
});
467-
return flag;
455+
NodePtr tab_under_mouse = nullptr;
456+
TraversalAccessible(
457+
page_tab_pane, [&tab_under_mouse, &pt](const NodePtr& child) {
458+
if (GetAccessibleRole(child) != ROLE_SYSTEM_PAGETAB) {
459+
return false;
460+
}
461+
GetAccessibleSize(child, [&tab_under_mouse, &pt, &child](RECT rect) {
462+
if (PtInRect(&rect, pt)) {
463+
tab_under_mouse = child;
464+
}
465+
});
466+
return tab_under_mouse != nullptr;
467+
});
468+
return tab_under_mouse;
469+
}
470+
471+
// Whether the mouse is on a tab
472+
bool IsOnOneTab(const NodePtr& top, POINT pt) {
473+
return GetTabUnderMouse(top, pt) != nullptr;
468474
}
469475

470476
bool IsOnlyOneTab(const NodePtr& top) {
@@ -587,16 +593,16 @@ bool IsOmniboxFocus(const NodePtr& top) {
587593
}
588594

589595
// Whether the mouse is on the close button of a tab.
590-
// Should be used together with `IsOnOneTab` to search the close button.
591-
bool IsOnCloseButton(const NodePtr& top, POINT pt) {
592-
if (!top) {
596+
// Pass the specific tab node to search the close button within that tab only.
597+
bool IsOnCloseButton(const NodePtr& node, POINT pt) {
598+
if (!node) {
593599
return false;
594600
}
595601

596602
bool found = false;
597-
auto find_hit_button = [&](this auto&& self, const NodePtr& node) -> bool {
598-
if (GetAccessibleRole(node) == ROLE_SYSTEM_PUSHBUTTON) {
599-
GetAccessibleSize(node, [&](RECT rect) {
603+
auto find_hit_button = [&](this auto&& self, const NodePtr& child) -> bool {
604+
if (GetAccessibleRole(child) == ROLE_SYSTEM_PUSHBUTTON) {
605+
GetAccessibleSize(child, [&](RECT rect) {
600606
if (PtInRect(&rect, pt)) {
601607
found = true;
602608
}
@@ -605,10 +611,10 @@ bool IsOnCloseButton(const NodePtr& top, POINT pt) {
605611
return true;
606612
}
607613
}
608-
TraversalAccessible(node, self);
614+
TraversalAccessible(child, self);
609615
return found;
610616
};
611-
TraversalAccessible(top, find_hit_button, false);
617+
TraversalAccessible(node, find_hit_button, false);
612618
return found;
613619
}
614620

src/iaccessible.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ using NodePtr = Microsoft::WRL::ComPtr<IAccessible>;
99
NodePtr GetChromeWidgetWin(HWND hwnd);
1010
NodePtr GetTopContainerView(HWND hwnd);
1111
int GetTabCount(const NodePtr& top);
12+
NodePtr GetTabUnderMouse(const NodePtr& top, POINT pt);
1213
bool IsOnOneTab(const NodePtr& top, POINT pt);
1314
bool IsOnlyOneTab(const NodePtr& top);
1415
bool IsOnTheTabBar(const NodePtr& top, POINT pt);

src/tabbookmark.cc

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,13 @@ bool HandleDoubleClick(const MOUSEHOOKSTRUCT* pmouse) {
104104
return false;
105105
}
106106

107-
bool is_on_one_tab = IsOnOneTab(top_container_view, pt);
108-
bool is_on_close_button = IsOnCloseButton(top_container_view, pt);
109-
if (!is_on_one_tab || is_on_close_button) {
107+
const NodePtr tab_under_mouse = GetTabUnderMouse(top_container_view, pt);
108+
if (!tab_under_mouse) {
109+
return false;
110+
}
111+
112+
// Avoid triggering on close button clicks.
113+
if (IsOnCloseButton(tab_under_mouse, pt)) {
110114
return false;
111115
}
112116

@@ -179,8 +183,11 @@ bool HandleCloseButton(const MOUSEHOOKSTRUCT* pmouse) {
179183
return false;
180184
}
181185

182-
if (!IsOnOneTab(top_container_view, pt) ||
183-
!IsOnCloseButton(top_container_view, pt)) {
186+
// Get the tab under mouse and check if close button is clicked within it.
187+
// This prevents false positives from other PUSHBUTTON elements like
188+
// "New Tab" button.
189+
const NodePtr tab_under_mouse = GetTabUnderMouse(top_container_view, pt);
190+
if (!tab_under_mouse || !IsOnCloseButton(tab_under_mouse, pt)) {
184191
return false;
185192
}
186193

0 commit comments

Comments
 (0)