diff --git a/tests/apps/Makefile.wasm b/tests/apps/Makefile.wasm index 179c0c9..313c97d 100644 --- a/tests/apps/Makefile.wasm +++ b/tests/apps/Makefile.wasm @@ -985,3 +985,18 @@ secondary-glcanvas: $(S)/secondary-glcanvas/secondary-glcanvas_test.html # Include the new app in the default `all` build (prerequisites accumulate). all: $(S)/secondary-glcanvas/secondary-glcanvas_test.html + +# --- secondary-frame chrome probe (GL-enabled) -------------------------------- +# Isolation harness for the "secondary top-level frame has no usable title bar" +# behaviour (KiCad 3D viewer / footprint editor). Reuses LDFLAGS_GL defined in the +# secondary-glcanvas block above. See tests/e2e/secondary-frame-chrome.spec.ts. +$(S)/secondary-frame-chrome/secondary-frame-chrome_test.o: $(S)/secondary-frame-chrome/secondary-frame-chrome_test.cpp + $(CXX) -c $(CXXFLAGS) $< -o $@ + +$(S)/secondary-frame-chrome/secondary-frame-chrome_test.html: $(S)/secondary-frame-chrome/secondary-frame-chrome_test.o $(WX_CORE_LIB) $(JS_FILES) + $(CXX) $< $(LDFLAGS_GL) --pre-js $(JS) --shell-file $(HTML) -o $@ + +secondary-frame-chrome: $(S)/secondary-frame-chrome/secondary-frame-chrome_test.html +.PHONY: secondary-frame-chrome + +all: $(S)/secondary-frame-chrome/secondary-frame-chrome_test.html diff --git a/tests/apps/standalone/secondary-frame-chrome/secondary-frame-chrome_test.cpp b/tests/apps/standalone/secondary-frame-chrome/secondary-frame-chrome_test.cpp new file mode 100644 index 0000000..7748221 --- /dev/null +++ b/tests/apps/standalone/secondary-frame-chrome/secondary-frame-chrome_test.cpp @@ -0,0 +1,215 @@ +// Isolation harness for the "secondary top-level frame has no usable title bar" +// behaviour seen with KiCad's 3D viewer / footprint editor in the WASM DOM port. +// +// Background: the wx DOM port draws a NON-main top-level window's chrome — a 22px +// title bar with a drag region + an "X" (the canvas-drawn minimize button) — onto +// the window's OWN 2D canvas, gated on HasTitleBar() == !IsMainFrame() (see +// src/wasm/toplevel.cpp). The main frame (wxTopLevelWindows[0]) has no chrome (the +// page is its window). KiCad's 3D viewer is a SECONDARY top-level wxFrame +// (non-main), sized wxDefaultSize -> full screen, carrying a wxGLCanvas that +// createGLCanvas lifts to z-index 2147483647 (above its own chrome). By the code +// such a frame SHOULD have a working title bar; in practice it can't be dragged or +// closed. This app reproduces those conditions in pure wxWidgets so the behaviour +// can be probed in isolation (no KiCad, no Docker). +// +// Buttons open secondary windows in three configs for A/B comparison: +// - Full GL Frame : wxDefaultSize (full screen) + wxGLCanvas (faithful 3D viewer) +// - Small Frame : fixed size, no GL (chrome-only control) +// - Modeless Dialog: fixed-size wxDialog (known-working chrome) +// +// Pair with the temporary [TLW] logging in src/wasm/toplevel.cpp, which reports +// HasTitleBar / NC paint / title-bar mouse hits per window, and the e2e spec +// tests/e2e/secondary-frame-chrome.spec.ts which drives the drag/close probes. + +#include "wx/wxprec.h" + +#ifndef WX_PRECOMP + #include "wx/wx.h" +#endif + +#include "wx/glcanvas.h" +#include "wx/artprov.h" +#include "wx/toolbar.h" + +#include + +#ifdef __EMSCRIPTEN__ +#include +#endif + +namespace +{ +// Default-attribute wxGLCanvas; the wasm port's ConvertWXAttrsToWebGL fills in +// WebGL2 + preserveDrawingBuffer. No GL drawing needed — createGLCanvas (which +// assigns the z-index) runs at construction, like KiCad's EDA_3D_CANVAS. +wxGLCanvas* MakeGLCanvas( wxWindow* parent ) +{ + wxGLAttributes attrs; + attrs.PlatformDefaults().Defaults().EndList(); + return new wxGLCanvas( parent, attrs, wxID_ANY, wxDefaultPosition, wxDefaultSize ); +} + +void LogWindow( const char* tag, wxTopLevelWindow* w ) +{ + const wxSize sz = w->GetSize(); + printf( "[CHROME-TEST] %s: IsMainFrame=%d size=%dx%d\n", + tag, (int) w->IsMainFrame(), sz.x, sz.y ); + fflush( stdout ); +} +} // namespace + +// Faithful 3D-viewer analog: a SECONDARY top-level frame, default-sized (resolved +// to full screen in the wasm port) with a wxGLCanvas filling its client area plus +// a status bar (the 3D viewer has one). +class FullGLFrame : public wxFrame +{ +public: + FullGLFrame() + : wxFrame( nullptr, wxID_ANY, "Full GL Frame" ) + { + CreateStatusBar(); + SetStatusText( "secondary GL frame" ); + + wxBoxSizer* sizer = new wxBoxSizer( wxVERTICAL ); + sizer->Add( MakeGLCanvas( this ), 1, wxEXPAND ); + SetSizer( sizer ); + + LogWindow( "FullGLFrame", this ); + } +}; + +// Richer 3D-viewer analog: adds the chrome the real EDA_3D_VIEWER_FRAME carries — +// a menu bar, a top toolbar, a left side panel (APPEARANCE_CONTROLS_3D), a status +// bar — on top of a full-size GL canvas. Tests whether any of those DOM elements +// steals the title-bar (NC) region's clicks. +class RichGLFrame : public wxFrame +{ +public: + RichGLFrame() + : wxFrame( nullptr, wxID_ANY, "Rich GL Frame" ) + { + wxMenuBar* menuBar = new wxMenuBar(); + wxMenu* fileMenu = new wxMenu(); + fileMenu->Append( wxID_EXIT, "E&xit" ); + menuBar->Append( fileMenu, "&File" ); + SetMenuBar( menuBar ); + + wxToolBar* toolBar = CreateToolBar(); + toolBar->AddTool( wxID_ZOOM_IN, "Zoom", wxArtProvider::GetBitmap( wxART_PLUS ) ); + toolBar->AddTool( wxID_HOME, "Reset", wxArtProvider::GetBitmap( wxART_GO_HOME ) ); + toolBar->Realize(); + + CreateStatusBar(); + SetStatusText( "rich secondary GL frame" ); + + wxBoxSizer* sizer = new wxBoxSizer( wxHORIZONTAL ); + wxPanel* side = new wxPanel( this, wxID_ANY, wxDefaultPosition, wxSize( 180, -1 ) ); + new wxStaticText( side, wxID_ANY, "Appearance", wxPoint( 8, 8 ) ); + sizer->Add( side, 0, wxEXPAND ); + sizer->Add( MakeGLCanvas( this ), 1, wxEXPAND ); + SetSizer( sizer ); + + LogWindow( "RichGLFrame", this ); + } +}; + +// Chrome-only control: a fixed-size secondary frame with no GL canvas. +class SmallFrame : public wxFrame +{ +public: + SmallFrame() + : wxFrame( nullptr, wxID_ANY, "Small Frame", wxPoint( 180, 300 ), wxSize( 360, 280 ) ) + { + wxPanel* panel = new wxPanel( this ); + new wxStaticText( panel, wxID_ANY, "Small secondary frame (no GL)", wxPoint( 16, 16 ) ); + + LogWindow( "SmallFrame", this ); + } +}; + +// Known-working baseline: a modeless wxDialog (same chrome path as Preferences / +// Print, which the user reports ARE draggable and X-closable). +class BaselineDialog : public wxDialog +{ +public: + explicit BaselineDialog( wxWindow* parent ) + : wxDialog( parent, wxID_ANY, "Modeless Dialog", wxPoint( 560, 300 ), wxSize( 360, 280 ) ) + { + new wxStaticText( this, wxID_ANY, "Dialog: drag + X should work", wxPoint( 16, 16 ) ); + + LogWindow( "BaselineDialog", this ); + } +}; + +class MainFrame : public wxFrame +{ +public: + MainFrame() + : wxFrame( nullptr, wxID_ANY, "Main Frame", wxDefaultPosition, wxSize( 800, 600 ) ) + { + wxPanel* panel = new wxPanel( this ); + wxBoxSizer* sizer = new wxBoxSizer( wxVERTICAL ); + + auto addButton = + [&]( const wxString& label, void ( MainFrame::*handler )( wxCommandEvent& ) ) + { + wxButton* button = new wxButton( panel, wxID_ANY, label ); + button->Bind( wxEVT_BUTTON, handler, this ); + sizer->Add( button, 0, wxALL, 8 ); + }; + + addButton( "Open Full GL Frame", &MainFrame::OnFullGL ); + addButton( "Open Rich GL Frame", &MainFrame::OnRichGL ); + addButton( "Open Small Frame", &MainFrame::OnSmall ); + addButton( "Open Modeless Dialog", &MainFrame::OnDialog ); + + // A GL canvas in the MAIN frame too, so a secondary GL canvas is created + // "while another GL canvas is already visible" — the condition that lifts + // it to z-index 2147483647 over the chrome (mirrors pcbnew's GAL canvas). + sizer->Add( MakeGLCanvas( panel ), 1, wxEXPAND ); + panel->SetSizer( sizer ); + + LogWindow( "MainFrame", this ); + } + +private: + void OnFullGL( wxCommandEvent& WXUNUSED( evt ) ) { ( new FullGLFrame() )->Show( true ); } + void OnRichGL( wxCommandEvent& WXUNUSED( evt ) ) { ( new RichGLFrame() )->Show( true ); } + void OnSmall( wxCommandEvent& WXUNUSED( evt ) ) { ( new SmallFrame() )->Show( true ); } + void OnDialog( wxCommandEvent& WXUNUSED( evt ) ) { ( new BaselineDialog( this ) )->Show( true ); } +}; + +class SecondaryFrameChromeApp : public wxApp +{ +public: + bool OnInit() override + { + if( !wxApp::OnInit() ) + return false; + +#ifdef __EMSCRIPTEN__ + // The bare test shell (template.html) lays out #window-container BELOW the + // full-size main #canvas in normal flow, so secondary windows render off + // the bottom of the page (and body overflow is hidden, so you can't scroll + // to them). The real standalone app overlays it. Overlay it here so the + // opened frames are visible for MANUAL interaction. The window divs are + // pointer-events:none and this container is 0x0, so it does not block + // clicks to the main canvas or steal events. (Does not affect the + // Playwright probe, which uses wx-screen coords via #canvas.) + EM_ASM( { + var wc = document.getElementById( 'window-container' ); + if( wc ) + { + wc.style.position = 'absolute'; + wc.style.top = '0px'; + wc.style.left = '0px'; + } + } ); +#endif + + ( new MainFrame() )->Show( true ); + return true; + } +}; + +wxIMPLEMENT_APP( SecondaryFrameChromeApp ); diff --git a/tests/e2e/modal.spec.ts b/tests/e2e/modal.spec.ts index 523db29..62e94a3 100644 --- a/tests/e2e/modal.spec.ts +++ b/tests/e2e/modal.spec.ts @@ -128,6 +128,20 @@ test.describe('Modal dialog border + drag (pcbjam #22)', () => { expect(await tryLoadApp(page), 'App should load').toBe(true); await waitForRegistry(page); + // The bare test shell lays out #window-container BELOW the full-size main + // canvas, so the modal (and its DOM title bar) render off the bottom of the + // viewport — the real app shells overlay it. Overlay it here so the DOM title + // bar is reachable by the pointer (the old canvas bar was reached indirectly + // via #canvas registry coords; the DOM bar must be clicked where it renders). + await page.evaluate(() => { + const wc = document.getElementById('window-container'); + if (wc) { + wc.style.position = 'absolute'; + wc.style.top = '0'; + wc.style.left = '0'; + } + }); + await clickByLabel(page, 'Custom Dialog'); await waitForModalRect(page); // wait for the .window.toplevel to exist await page.waitForTimeout(400); @@ -147,12 +161,16 @@ test.describe('Modal dialog border + drag (pcbjam #22)', () => { ); await page.screenshot({ path: 'test-results/modal-02-before-drag.png', fullPage: true }); - // Grab the title-bar strip (top TITLE_BAR_HEIGHT=22px), away from the close - // button at top-right. The wasm pointer-move handler is asyncified, so dwell - // after the first move to let the hovered window settle before pressing - // (same pattern the GAL draw tests need). - const startX = dlgBefore.centerX; - const startY = dlgBefore.screenY + 8; + // The dialog now drags via its real DOM title bar (`.window-titlebar`): + // pointer events on it → wx_window_move → wxWindow::Move. So grab the element + // at its actual on-screen position (getBoundingClientRect), NOT the registry + // coords the old canvas title bar needed. Center is over the title text, clear + // of the close × at the right. + const titlebar = page.locator(`${MODAL_SEL} .window-titlebar`); + const tbox = await titlebar.boundingBox(); + expect(tbox, 'modal should have a DOM title bar').not.toBeNull(); + const startX = tbox!.x + tbox!.width / 2; + const startY = tbox!.y + tbox!.height / 2; await page.mouse.move(startX, startY); await page.waitForTimeout(350); await page.mouse.down(); diff --git a/tests/e2e/secondary-frame-chrome.spec.ts b/tests/e2e/secondary-frame-chrome.spec.ts new file mode 100644 index 0000000..5a8baf1 --- /dev/null +++ b/tests/e2e/secondary-frame-chrome.spec.ts @@ -0,0 +1,100 @@ +import { test, expect, waitForApp } from './utils/fixtures'; +import { clickByLabel, waitForRegistry } from './utils/element-tracker'; + +/** + * Validates the real-DOM title bar for secondary (non-main) wxFrames in the WASM + * DOM port — the fix for the "3D-viewer can't be dragged / can't be X-closed" bug. + * + * Root cause (confirmed): a non-main window's title bar used to be canvas-painted + * and pointer-events:none, so its clicks had to reach the central #canvas mouse + * router; an overlapping pointer-events:auto DOM control from another frame stole + * them. The fix gives ALL non-main top-level windows (secondary frames AND + * dialogs) a real DOM `.window-titlebar` (drag + `.window-titlebar-close`) that + * wins hit-testing via normal stacking. + * + * Drag dispatches real pointer events on the title bar (→ wx_window_move → + * wxWindow::Move); close clicks the × (→ wx_window_close → wx Close(); for a + * modal dialog this ends the modal loop via EndModal). + */ + +const URL = '/standalone/secondary-frame-chrome/secondary-frame-chrome_test.html'; + +test.describe('secondary-frame DOM title bar (drag / close)', () => { + test('frames and dialogs get a draggable, closable DOM title bar', async ({ page }) => { + await page.goto(URL); + await waitForApp(page); + await waitForRegistry(page); + + const listWindows = () => + page.evaluate(() => + Array.from(document.querySelectorAll('#window-container [id^="window-"]')).map((e) => e.id)); + + const styleRect = (id: string) => + page.evaluate((wid) => { + const el = document.getElementById(wid) as HTMLElement | null; + if (!el) return null; + const n = (v: string) => parseInt(v || '0', 10) || 0; + return { left: n(el.style.left), top: n(el.style.top), width: n(el.style.width), height: n(el.style.height) }; + }, id); + + async function openWindow(buttonLabel: string): Promise { + const before = await listWindows(); + expect(await clickByLabel(page, buttonLabel), `"${buttonLabel}" should be clickable`).toBe(true); + await page.waitForFunction( + (b: string[]) => Array.from(document.querySelectorAll('#window-container [id^="window-"]')).some((e) => !b.includes(e.id)), + before, { timeout: 15000 }); + const after = await listWindows(); + const id = after.find((w) => !before.includes(w)); + expect(id, `${buttonLabel} should open a new window`).toBeTruthy(); + await page.waitForTimeout(200); + return id as string; + } + + // Returns true if the window moved after a title-bar drag. + async function dragViaTitlebar(winId: string): Promise { + const bar = page.locator(`#${winId} .window-titlebar`); + const box = await bar.boundingBox(); + if (!box) return false; + const before = await styleRect(winId); + const sx = box.x + box.width / 2; + const sy = box.y + box.height / 2; + await page.mouse.move(sx, sy); + await page.mouse.down(); + await page.mouse.move(sx, sy + 90, { steps: 10 }); + await page.mouse.up(); + await page.waitForTimeout(250); + const after = await styleRect(winId); + return !!before && !!after && (Math.abs(after.top - before.top) > 5 || Math.abs(after.left - before.left) > 5); + } + + async function closeViaTitlebar(winId: string): Promise { + await page.locator(`#${winId} .window-titlebar-close`).click(); + await page.waitForTimeout(400); + return page.evaluate((wid) => { + const el = document.getElementById(wid); + return !el || getComputedStyle(el).display === 'none'; + }, winId); + } + + // All non-main top-level windows — secondary frames AND dialogs — must + // have a DOM title bar, be draggable by it, and close via its × button. + for (const label of [ + 'Open Full GL Frame', + 'Open Rich GL Frame', + 'Open Small Frame', + 'Open Modeless Dialog', + ]) { + const id = await openWindow(label); + const hasBar = await page.locator(`#${id} .window-titlebar`).count(); + expect(hasBar, `${label} should have a DOM title bar`).toBe(1); + + // Root-cause check: even with main-frame DOM controls present, the title + // bar is the top hit-test element at its own location. + const moved = await dragViaTitlebar(id); + expect(moved, `${label} should be draggable by its DOM title bar`).toBe(true); + + const closed = await closeViaTitlebar(id); + expect(closed, `${label} should close via its × button`).toBe(true); + } + }); +}); diff --git a/tests/kicad/3d-viewer.spec.ts b/tests/kicad/3d-viewer.spec.ts index 9ee3399..ebef4b3 100644 --- a/tests/kicad/3d-viewer.spec.ts +++ b/tests/kicad/3d-viewer.spec.ts @@ -284,4 +284,80 @@ test.describe('3D viewer from pcbnew', () => { .filter((l) => l.includes('Aborted(')); expect(aborts, `WASM aborted while opening the 3D viewer:\n${aborts.join('\n\n')}`).toEqual([]); }); + + /** + * Regression gate for "the 3D viewer can't be dragged / can't be X-closed". + * + * The viewer is a non-main wxFrame; it now gets a real DOM `.window-titlebar` + * (drag + `.window-titlebar-close`) instead of a canvas-painted, pointer- + * events:none bar whose clicks an overlapping main-frame DOM control stole. + * This drives the REAL viewer: it must have a DOM title bar, that bar must be + * the top hit-test element at its own location (even with the main editor's + * pointer-events:auto controls present), dragging it must move the frame, and + * the × must close it. + */ + test('real 3D viewer has a draggable, X-closable DOM title bar', async ({ page, testLogger }) => { + await page.goto('/kicad/pcbnew.html'); + await waitForPcbnew(page); + await loadBoard(page, testLogger); + + const winsBefore = await page.evaluate(() => + Array.from(document.querySelectorAll('#window-container [id^="window-"]')).map((e) => e.id)); + const glBefore = await countGlCanvases(page); + await openThreeDViewer(page, glBefore); + await page.waitForTimeout(1500); + + const winId = await page.evaluate((before: string[]) => { + const all = Array.from(document.querySelectorAll('#window-container [id^="window-"]')).map((e) => e.id); + return all.find((id) => !before.includes(id)) ?? all[all.length - 1] ?? null; + }, winsBefore); + expect(winId, 'the 3D viewer should open a new top-level window').toBeTruthy(); + await page.screenshot({ path: 'test-results/3d-viewer-titlebar.png', scale: 'device' }); + + // It must have a real DOM title bar (the frames-only fix covers the viewer). + const bar = page.locator(`#${winId} .window-titlebar`); + expect(await bar.count(), 'the 3D viewer (a wxFrame) should have a DOM title bar').toBe(1); + + // Root-cause check: the title bar is the top hit-test element at its own + // location, despite the main pcbnew frame's pointer-events:auto controls + // (which used to intercept and break drag/close). + const box = await bar.boundingBox(); + expect(box, 'the title bar should have a layout box').not.toBeNull(); + const cx = box!.x + box!.width / 2; + const cy = box!.y + box!.height / 2; + const topEl = await page.evaluate(([x, y]) => { + const el = document.elementFromPoint(x, y) as HTMLElement | null; + return el ? el.className.toString() : 'null'; + }, [cx, cy]); + expect(topEl, `the top element at the title bar must be the title bar (was "${topEl}")`) + .toContain('window-titlebar'); + + // Drag the title bar → the frame moves (wx_window_move → wxWindow::Move). + const styleTop = (wid: string) => + page.evaluate((id) => { + const el = document.getElementById(id) as HTMLElement | null; + return el ? (parseInt(el.style.top || '0', 10) || 0) : null; + }, wid); + const beforeTop = await styleTop(winId as string); + await page.mouse.move(cx, cy); + await page.mouse.down(); + await page.mouse.move(cx, cy + 80, { steps: 10 }); + await page.mouse.up(); + await page.waitForTimeout(300); + const afterTop = await styleTop(winId as string); + expect(afterTop, 'dragging the title bar should move the 3D viewer frame').not.toBe(beforeTop); + + // Close via the × (wx_window_close → wx Close() → OnCloseWindow). + await page.locator(`#${winId} .window-titlebar-close`).click(); + await page.waitForTimeout(600); + const gone = await page.evaluate((wid) => { + const el = document.getElementById(wid); + return !el || getComputedStyle(el).display === 'none'; + }, winId); + expect(gone, 'the × should close the 3D viewer').toBe(true); + + // Opening/dragging/closing didn't blow up the runtime. + const aborts = [...testLogger.consoleLogs, ...testLogger.errors].filter((l) => l.includes('Aborted(')); + expect(aborts, `WASM aborted during the title-bar test:\n${aborts.join('\n\n')}`).toEqual([]); + }); }); diff --git a/web/pcbjam-shared b/web/pcbjam-shared index 9a1a269..c6a7e00 160000 --- a/web/pcbjam-shared +++ b/web/pcbjam-shared @@ -1 +1 @@ -Subproject commit 9a1a269fdd9079921029ecf6e2dce89dece98fc8 +Subproject commit c6a7e00bcaa39023be0be126a28f8f1acb9a2d67 diff --git a/wxwidgets b/wxwidgets index 67f28fb..2152871 160000 --- a/wxwidgets +++ b/wxwidgets @@ -1 +1 @@ -Subproject commit 67f28fb3354a1830f004c989db9638205816c665 +Subproject commit 2152871eba88232b60bc424460c73f7c406a3837