From 4c199dff9f366268c0526659f22efbeaa9816034 Mon Sep 17 00:00:00 2001 From: Istvan Matejcsok <119620946+matejcsok-ee@users.noreply.github.com> Date: Fri, 19 Jun 2026 14:32:01 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20=F0=9F=90=9B=20tooltip=20+=20test=20case?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ✅ Closes: #18, #16 --- kicad | 2 +- tests/apps/Makefile.wasm | 10 ++ .../tooltip-toolbar/tooltip-toolbar_test.cpp | 120 ++++++++++++++++++ tests/e2e/dom-port-bugs.spec.ts | 76 ++++++++++- wxwidgets | 2 +- 5 files changed, 207 insertions(+), 3 deletions(-) create mode 100644 tests/apps/standalone/tooltip-toolbar/tooltip-toolbar_test.cpp diff --git a/kicad b/kicad index f7a15ad..c77a4af 160000 --- a/kicad +++ b/kicad @@ -1 +1 @@ -Subproject commit f7a15ade476dec8615b864cbb90ef40662537b7e +Subproject commit c77a4aff28cc4710f773bf0284c8ad2e4732895a diff --git a/tests/apps/Makefile.wasm b/tests/apps/Makefile.wasm index a87de2e..03cb31c 100644 --- a/tests/apps/Makefile.wasm +++ b/tests/apps/Makefile.wasm @@ -196,6 +196,7 @@ all: minimal_test.html \ $(S)/textctrl-reentry/textctrl-reentry_test.html \ $(S)/stattext-ellipsize/stattext-ellipsize_test.html \ $(S)/tooltip-lifetime/tooltip-lifetime_test.html \ + $(S)/tooltip-toolbar/tooltip-toolbar_test.html \ $(S)/warp-pointer/warp-pointer_test.html # Main test app @@ -235,6 +236,15 @@ $(S)/tooltip-lifetime/tooltip-lifetime_test.html: $(S)/tooltip-lifetime/tooltip- tooltip-lifetime: $(S)/tooltip-lifetime/tooltip-lifetime_test.html +# wxAuiToolBar per-tool tooltip repro (dom-port-bugs.spec.ts). +$(S)/tooltip-toolbar/tooltip-toolbar_test.o: $(S)/tooltip-toolbar/tooltip-toolbar_test.cpp + $(CXX) -c $(CXXFLAGS) $< -o $@ + +$(S)/tooltip-toolbar/tooltip-toolbar_test.html: $(S)/tooltip-toolbar/tooltip-toolbar_test.o $(WX_CORE_LIB) $(JS_FILES) + $(CXX) $< $(LDFLAGS_NOGL) --pre-js $(JS) --shell-file $(HTML) -o $@ + +tooltip-toolbar: $(S)/tooltip-toolbar/tooltip-toolbar_test.html + # wxWindow::WarpPointer must update the cached mouse position (wxGetMousePosition). $(S)/warp-pointer/warp-pointer_test.o: $(S)/warp-pointer/warp-pointer_test.cpp $(CXX) -c $(CXXFLAGS) $< -o $@ diff --git a/tests/apps/standalone/tooltip-toolbar/tooltip-toolbar_test.cpp b/tests/apps/standalone/tooltip-toolbar/tooltip-toolbar_test.cpp new file mode 100644 index 0000000..5639de0 --- /dev/null +++ b/tests/apps/standalone/tooltip-toolbar/tooltip-toolbar_test.cpp @@ -0,0 +1,120 @@ +// wxAuiToolBar per-tool tooltip reproduction (DOM port). +// +// KiCad's ACTION_TOOLBAR derives from wxAuiToolBar, whose tool buttons are +// painted "islands" inside ONE wxWindow. Tooltips for painted windows are +// driven by the C++ hover layer (src/wasm/tooltip.cpp), armed from +// wxApp::HandleMouseEvent ONLY when the hovered wxWindow changes — and BEFORE +// the motion is dispatched to the toolbar. Two failures result: +// +// 1. Entering the toolbar arms the tooltip timer before wxAuiToolBar::OnMotion +// has set the hovered tool's short-help as the toolbar's tooltip, so the +// layer reads empty text and often never shows anything ("not always +// shows"). +// 2. Moving between two tools on the SAME toolbar never changes g_mouseWindow, +// so the layer is never re-armed and the tooltip never updates ("doesn't +// update when moving to another button"). +// +// This app lays out a wxAuiToolBar with three tools (TOOLTIP_A/B/C) and logs +// each tool's #canvas-relative rect. The e2e (dom-port-bugs.spec.ts) moves the +// real pointer over the tools and asserts the #wx-tooltip element shows and +// updates: RED before the wasm-layer fix, GREEN after. + +#include "wx/wxprec.h" +#ifndef WX_PRECOMP + #include "wx/wx.h" +#endif + +#include "wx/aui/aui.h" +#include "wx/artprov.h" + +#ifdef __EMSCRIPTEN__ +#include +#endif + +enum +{ + ID_TOOL_A = wxID_HIGHEST + 1, + ID_TOOL_B, + ID_TOOL_C +}; + +class ReproFrame : public wxFrame +{ +public: + ReproFrame(); + +private: + wxAuiToolBar *m_toolbar; + + void EmitLayout(); + void LogToolRect(const char *name, int toolId); +}; + +ReproFrame::ReproFrame() + : wxFrame(nullptr, wxID_ANY, "wxAuiToolBar tooltip repro", + wxDefaultPosition, wxSize(640, 400)) +{ + m_toolbar = new wxAuiToolBar(this, wxID_ANY, wxDefaultPosition, + wxDefaultSize, wxAUI_TB_HORIZONTAL); + + // short-help string == the per-tool tooltip text wxAuiToolBar::OnMotion + // pushes onto itself via SetToolTip(). + m_toolbar->AddTool(ID_TOOL_A, "A", + wxArtProvider::GetBitmap(wxART_NEW, wxART_TOOLBAR), "TOOLTIP_A"); + m_toolbar->AddTool(ID_TOOL_B, "B", + wxArtProvider::GetBitmap(wxART_FILE_OPEN, wxART_TOOLBAR), "TOOLTIP_B"); + m_toolbar->AddTool(ID_TOOL_C, "C", + wxArtProvider::GetBitmap(wxART_FILE_SAVE, wxART_TOOLBAR), "TOOLTIP_C"); + m_toolbar->Realize(); + + wxBoxSizer *sizer = new wxBoxSizer(wxVERTICAL); + sizer->Add(m_toolbar, 0, wxEXPAND); + sizer->Add(new wxPanel(this, wxID_ANY), 1, wxEXPAND); + SetSizer(sizer); + Layout(); + + // Emit tool rects once layout has settled. + CallAfter(&ReproFrame::EmitLayout); +} + +void ReproFrame::LogToolRect(const char *name, int toolId) +{ + const wxRect r = m_toolbar->GetToolRect(toolId); + const wxPoint tl = m_toolbar->ClientToScreen(r.GetTopLeft()); + +#ifdef __EMSCRIPTEN__ + // #canvas-relative (wx "screen") coords; the e2e adds the #canvas origin. + EM_ASM({ + console.log('[REPRO] toolrect ' + UTF8ToString($0) + ' ' + + $1 + ' ' + $2 + ' ' + $3 + ' ' + $4); + }, name, tl.x, tl.y, r.GetWidth(), r.GetHeight()); +#else + (void)name; (void)tl; (void)r; +#endif +} + +void ReproFrame::EmitLayout() +{ + LogToolRect("TOOLTIP_A", ID_TOOL_A); + LogToolRect("TOOLTIP_B", ID_TOOL_B); + LogToolRect("TOOLTIP_C", ID_TOOL_C); + +#ifdef __EMSCRIPTEN__ + EM_ASM({ console.log('[REPRO] tooltip-toolbar ready'); }); +#endif +} + +class ReproApp : public wxApp +{ +public: + bool OnInit() override + { + if (!wxApp::OnInit()) + return false; + + (new ReproFrame())->Show(true); + return true; + } +}; + +wxIMPLEMENT_APP(ReproApp); diff --git a/tests/e2e/dom-port-bugs.spec.ts b/tests/e2e/dom-port-bugs.spec.ts index f82e5e2..f335d58 100644 --- a/tests/e2e/dom-port-bugs.spec.ts +++ b/tests/e2e/dom-port-bugs.spec.ts @@ -1,4 +1,4 @@ -import { test, expect, tryLoadApp } from './utils/fixtures'; +import { test, expect, tryLoadApp, getCanvasBox } from './utils/fixtures'; // Red-green reproductions for the two DOM-port bugs in // docs/features/wx-dom-port/branch-review.md. @@ -115,4 +115,78 @@ test.describe('wx DOM-port bug reproductions', () => { }) .toBe(true); }); + + // KiCad's ACTION_TOOLBAR derives from wxAuiToolBar, whose tool buttons are + // painted "islands" inside ONE wxWindow. The C++ hover tooltip layer + // (src/wasm/tooltip.cpp) is armed from wxApp::HandleMouseEvent only when the + // hovered wxWindow changes, and BEFORE the motion is dispatched — so (1) the + // first tool's tooltip often never arms (read before wxAuiToolBar::OnMotion + // sets it) and (2) moving between tools on the same toolbar never re-arms. + // The app logs each tool's #canvas-relative rect; we drive the real pointer + // and assert the #wx-tooltip layer shows the hovered tool's text and updates. + test('wxAuiToolBar: tooltip shows on hover and updates when moving between tools', async ({ + page, + testLogger, + }) => { + await page.goto('/standalone/tooltip-toolbar/tooltip-toolbar_test.html'); + expect(await tryLoadApp(page, 30000), 'repro app should load').toBe(true); + + await expect + .poll(() => testLogger.consoleLogs.some((l) => l.includes('[REPRO] tooltip-toolbar ready')), { + timeout: 30000, + message: 'repro app should finish setup', + }) + .toBe(true); + + // Tool rects (in #canvas-relative coords) logged by the app. + const rects: Record = {}; + for (const l of testLogger.consoleLogs) { + const m = l.match(/\[REPRO\] toolrect (\S+) (-?\d+) (-?\d+) (-?\d+) (-?\d+)/); + if (m) rects[m[1]] = { x: +m[2], y: +m[3], w: +m[4], h: +m[5] }; + } + expect(rects['TOOLTIP_A'], 'tool A rect logged').toBeTruthy(); + expect(rects['TOOLTIP_B'], 'tool B rect logged').toBeTruthy(); + + const canvas = await getCanvasBox(page); + const center = (r: { x: number; y: number; w: number; h: number }) => ({ + x: canvas.x + r.x + r.w / 2, + y: canvas.y + r.y + r.h / 2, + }); + const a = center(rects['TOOLTIP_A']); + const b = center(rects['TOOLTIP_B']); + + const readTooltip = () => + page.evaluate(() => { + const el = document.getElementById('wx-tooltip'); + if (!el) return { visible: false, text: '' }; + const visible = el.style.display !== 'none' && getComputedStyle(el).display !== 'none'; + return { visible, text: el.textContent || '' }; + }); + const shownText = async () => { + const t = await readTooltip(); + return t.visible ? t.text : ''; + }; + + // Settle the pointer off the toolbar, then hover tool A. + await page.mouse.move(canvas.x + canvas.width / 2, canvas.y + canvas.height - 20); + await page.mouse.move(a.x, a.y); + + // RED today: the tooltip often never arms for the first hovered tool. + await expect + .poll(shownText, { + timeout: 4000, + message: 'tooltip should show TOOLTIP_A when hovering tool A', + }) + .toBe('TOOLTIP_A'); + + // Move to tool B (same toolbar window). RED today: never re-armed, so it + // stays on TOOLTIP_A or hides. + await page.mouse.move(b.x, b.y); + await expect + .poll(shownText, { + timeout: 4000, + message: 'tooltip should update to TOOLTIP_B when moving to tool B', + }) + .toBe('TOOLTIP_B'); + }); }); diff --git a/wxwidgets b/wxwidgets index 97359f9..0f48716 160000 --- a/wxwidgets +++ b/wxwidgets @@ -1 +1 @@ -Subproject commit 97359f9331fcd60e6d005534dd2ed09af8909901 +Subproject commit 0f487162ab567353ee7c43d9bd9d7108bc1ca52c