From d15098fce61c94d88025a310f3d2929fd85cc050 Mon Sep 17 00:00:00 2001 From: Istvan Matejcsok <119620946+matejcsok-ee@users.noreply.github.com> Date: Fri, 3 Jul 2026 12:21:58 +0200 Subject: [PATCH 01/14] =?UTF-8?q?test(kicad):=20C-1=20e2e=20=E2=80=94=20pr?= =?UTF-8?q?edefined=20via=20size=20sets=20diameter,=20not=200?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drives pcbnew's Track & Via Properties dialog: seed predefined via sizes via (setup (user_via ...)), Ctrl+A the sole via, pick a size, assert the diameter field. RED (NULL client data -> 0) / GREEN (0.9). Bumps wxwidgets to the C-1 fix. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/kicad/via-clientdata.spec.ts | 235 +++++++++++++++++++++++++++++ wxwidgets | 2 +- 2 files changed, 236 insertions(+), 1 deletion(-) create mode 100644 tests/kicad/via-clientdata.spec.ts diff --git a/tests/kicad/via-clientdata.spec.ts b/tests/kicad/via-clientdata.spec.ts new file mode 100644 index 000000000..9c44d1cfc --- /dev/null +++ b/tests/kicad/via-clientdata.spec.ts @@ -0,0 +1,235 @@ +import { test, expect } from './fixtures'; +import { clickByLabel } from '../e2e/utils/element-tracker'; +import type { Page } from '@playwright/test'; + +// KiCad-level reproduction of the wxWidgets DOM-port "selection events carry no +// per-item client data" bug (parity audit C-1, the audit's single Critical), +// fixed in src/wasm/{choice,combobox,listbox}.cpp by calling +// InitCommandEventWithItems() before dispatch. +// +// pcbnew's Track & Via Properties dialog is the ONLY KiCad surface that reads +// per-item client data off a wxChoice *selection event* (an exhaustive sweep of +// the tree found exactly one such consumer). It populates the "predefined via +// sizes" choice with a VIA_DIMENSION* as client data +// (dialog_track_via_properties.cpp:767-781) and, on selection, does with NO null +// guard (dialog_track_via_properties.cpp:1674-1680): +// +// VIA_DIMENSION* v = static_cast( aEvent.GetClientData() ); +// m_viaDiameter.ChangeValue( v->m_Diameter ); // v is NULL when the bug is present +// m_viaDrill.ChangeValue( v->m_Drill ); +// +// With the DOM-port bug, event.GetClientData() is always NULL; in WASM a null +// deref at a small offset reads 0 (no trap — see memory wasm-null-deref-reads-zero), +// so picking a predefined via size sets the Via diameter/drill fields to 0 +// instead of the predefined value. +// +// RED (bug present): after picking "0.9 / 0.45", the Via diameter field -> 0 +// (or the module aborts). +// GREEN (fixed): the Via diameter field -> 0.9 (the predefined value). +// +// The predefined via sizes are seeded from the board file's +// (setup (user_via ) ...) so no Board Setup interaction is +// needed. The board contains ONLY the via, so Edit -> Select All selects exactly +// the via (satisfying the tracks/vias-only condition the Properties action needs) +// with no fragile canvas-pixel clicking. + +const DOC_DIR = `/home/kicad/documents`; +const PCB_PATH = `${DOC_DIR}/via_clientdata.kicad_pcb`; + +// A minimal board: two predefined via sizes (0.9/0.45 and 1.1/0.55 mm) plus a +// single via whose own size (0.7/0.35) is distinct from both predefined sizes +// and from 0 — so the diameter field visibly changes on selection. +const BOARD = `(kicad_pcb + (version 20241229) + (generator "pcbnew") + (generator_version "9.0") + (general (thickness 1.6)) + (paper "A4") + (layers + (0 "F.Cu" signal) + (2 "B.Cu" signal) + ) + (setup + (user_via 0.9 0.45) + (user_via 1.1 0.55) + ) + (net 0 "") + (via (at 100 100) (size 0.7) (drill 0.35) (layers "F.Cu" "B.Cu") (net 0) + (uuid "77777777-0000-0000-0000-000000000001")) +) +`; + +async function waitForPcbnew(page: Page): Promise { + await expect(page.locator('#canvas')).toBeVisible({ timeout: 90000 }); + await page.waitForFunction(() => !!window.wxElementRegistry, null, { timeout: 90000 }); + await page.waitForFunction( + () => { + const r = window.wxElementRegistry; + return !!r && r.findAll({}).length > 0; + }, + null, + { timeout: 150000 }, + ); + await page.waitForTimeout(2500); + // dismiss the first-run setup wizard (Next > … Finish); no-op if absent + for (let i = 0; i < 12; i++) { + const advanced = await clickByLabel(page, 'Next >'); + if (!advanced) break; + await page.waitForTimeout(400); + } + await clickByLabel(page, 'Finish'); + await page.waitForTimeout(800); + await page.waitForFunction( + () => { + const r = window.wxElementRegistry; + if (!r) return false; + return r.findAll({ visible: true }).some((el) => el.name === 'PcbFrame'); + }, + null, + { timeout: 90000 }, + ); + await page.waitForTimeout(1500); +} + +// Open the board directly through the embind bridge (present in pcbnew.wasm). +async function openBoard(page: Page): Promise { + await page.evaluate( + ({ dir, path, content }) => { + const w = window as unknown as { + FS: { mkdirTree(p: string): void; writeFile(p: string, d: string): void }; + Module: { kicadOpenFile(p: string): unknown }; + }; + try { + w.FS.mkdirTree(dir); + } catch { + /* exists */ + } + w.FS.writeFile(path, content); + w.Module.kicadOpenFile(path); + }, + { dir: DOC_DIR, path: PCB_PATH, content: BOARD }, + ); +} + +// Snapshot the id + option texts of every single-select vs the toolbar ones). +async function dumpSelects(page: Page) { + return page.evaluate(() => { + const w = window as unknown as { __vsc?: number }; + w.__vsc = w.__vsc ?? 0; + return Array.from(document.querySelectorAll('select:not([multiple])')).map((sel) => { + const s = sel as HTMLSelectElement; + if (!s.id) s.id = `__vsel_${w.__vsc!++}`; + return { id: s.id, options: Array.from(s.options).map((o) => o.text) }; + }); + }); +} + +// The bounding box of the visible GAL WebGL canvas (some glcanvas-* are hidden). +async function visibleGlCanvasBox(page: Page) { + const id = await page.evaluate(() => { + const c = Array.from(document.querySelectorAll('[id^="glcanvas-"]')) + .map((el) => el as HTMLCanvasElement) + .find((el) => { + const rect = el.getBoundingClientRect(); + return window.getComputedStyle(el).display !== 'none' && rect.width > 0 && rect.height > 0; + }); + return c?.id ?? null; + }); + expect(id, 'a visible GL canvas').not.toBeNull(); + const box = await page.locator(`#${id}`).boundingBox(); + expect(box, 'GL canvas bounding box').not.toBeNull(); + return box!; +} + +test.describe('pcbnew Track & Via Properties — selection-event client data (C-1)', () => { + test.describe.configure({ timeout: 240000 }); + + test('predefined via size sets the diameter field (not 0)', async ({ page, testLogger }) => { + await page.goto('/kicad/pcbnew.html'); + await waitForPcbnew(page); + + // Load the crafted board and let it settle. + await openBoard(page); + await page.waitForTimeout(3000); + await page.screenshot({ path: 'test-results/via-clientdata-00-loaded.png', scale: 'device' }); + + // Record the toolbar (not a toolbar one) whose options look + // like via sizes ("0.9 / 0.45 …") can only appear when the dialog opens. + await expect + .poll( + async () => { + return (await dumpSelects(page)).some( + (s) => !beforeSelIds.has(s.id) && s.options.some((o) => /\d\s*\/\s*\d/.test(o)), + ); + }, + { + timeout: 30000, + message: 'Track & Via Properties dialog with the predefined-via-size choice should open', + }, + ) + .toBe(true); + + await page.screenshot({ path: 'test-results/via-clientdata-01-dialog.png', scale: 'device' }); + + const selects = await dumpSelects(page); + const viaSelect = selects.find( + (s) => !beforeSelIds.has(s.id) && s.options.some((o) => /\d\s*\/\s*\d/.test(o)), + ); + expect(viaSelect, 'dialog predefined-via-size whose value ≈ 0.7 (the via's + // current diameter, distinct from every other field and from 0). + const diameterBefore = await page.evaluate(() => { + const inputs = Array.from(document.querySelectorAll('input')) as HTMLInputElement[]; + const hit = inputs.find((el) => Math.abs(parseFloat(el.value) - 0.7) < 0.02); + if (hit && !hit.id) hit.id = '__via_diameter_input'; + return hit ? { id: hit.id, value: hit.value } : null; + }); + console.log('[VIA] diameter field before: ' + JSON.stringify(diameterBefore)); + expect(diameterBefore, 'via diameter field (≈0.7) found before selection').toBeTruthy(); + + // Pick the predefined via size -> fires DOM change -> wxChoice::OnDomEvent -> + // onViaSelect reads event.GetClientData(). + await page.locator(`#${viaSelect!.id}`).selectOption({ label: targetOption }); + await page.waitForTimeout(1000); + + await page.screenshot({ path: 'test-results/via-clientdata-02-selected.png', scale: 'device' }); + + const diameterAfter = await page.evaluate((id: string) => { + const el = document.getElementById(id) as HTMLInputElement | null; + return el ? el.value : null; + }, diameterBefore!.id); + const numAfter = parseFloat(diameterAfter ?? 'NaN'); + console.log(`[VIA] diameter field after: "${diameterAfter}" (${numAfter})`); + + const aborted = [...testLogger.consoleLogs, ...testLogger.errors].some((l) => + l.includes('Aborted('), + ); + expect(aborted, 'the WASM module must not abort on the via-size selection').toBe(false); + + // RED (bug): diameter -> 0. GREEN (fixed): diameter -> 0.9 (the predefined value). + expect( + numAfter, + `predefined via size 0.9 mm must land in the diameter field (got "${diameterAfter}")`, + ).toBeCloseTo(0.9, 2); + }); +}); diff --git a/wxwidgets b/wxwidgets index d9c3feecd..bdafcc8c3 160000 --- a/wxwidgets +++ b/wxwidgets @@ -1 +1 @@ -Subproject commit d9c3feecddad2ac33fc27a217f1a32885d0c0823 +Subproject commit bdafcc8c3f3fe7d8ca078922a1f7c11e317e0c40 From 8999f05c14f2ddd5536ee562b65b5205d46a99d5 Mon Sep 17 00:00:00 2001 From: Istvan Matejcsok <119620946+matejcsok-ee@users.noreply.github.com> Date: Fri, 3 Jul 2026 12:22:17 +0200 Subject: [PATCH 02/14] =?UTF-8?q?test(kicad):=20H-1=20e2e=20=E2=80=94=20mo?= =?UTF-8?q?dal=20dialog=20disables=20the=20main=20pcbnew=20frame?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drives the Page Settings dialog (a real wxDialog::ShowModal — not the quasi-modal Plot/Track&Via, which already disable the parent via KiCad's WINDOW_DISABLER). Asserts the PcbFrame registry 'enabled' flips true->false->true across the modal. Measured RED true / GREEN false. Also routes the spec to chromium-ci. Bumps wxwidgets to the H-1 fix. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/kicad/modal-input-lock.spec.ts | 167 +++++++++++++++++++++++++++ tests/playwright-kicad.config.ts | 2 + 2 files changed, 169 insertions(+) create mode 100644 tests/kicad/modal-input-lock.spec.ts diff --git a/tests/kicad/modal-input-lock.spec.ts b/tests/kicad/modal-input-lock.spec.ts new file mode 100644 index 000000000..a7074778f --- /dev/null +++ b/tests/kicad/modal-input-lock.spec.ts @@ -0,0 +1,167 @@ +import { test, expect } from './fixtures'; +import { waitForPcbnew } from './utils/pcbnew-ready'; +import { clickMenuBarItem } from '../e2e/utils/element-tracker'; +import type { Page } from '@playwright/test'; + +// KiCad-level reproduction of the wxWidgets DOM-port "modal dialogs are not +// input-modal" bug (parity audit H-1), fixed in src/wasm/dialog.cpp by creating +// a wxWindowDisabler in wxDialog::ShowModal(). +// +// The audit claimed this could not be surfaced as a product test ("e2e tests +// drive only the dialog, so they don't surface it"). It can: +// +// A real modal dialog (wxDialog::ShowModal) is supposed to make the rest of the +// application input-inert until it closes. Native wxWidgets does this by creating +// a wxWindowDisabler, which calls Disable() on every OTHER top-level window. The +// DOM port dropped that one line (dialog.cpp: m_windowDisabler is NULL-inited and +// only ever wxDELETE'd, never new'd), so the parent pcbnew frame stays fully live +// behind a "modal" — its menubar/toolbar/canvas keep accepting input. +// +// The fix (restore `m_windowDisabler = new wxWindowDisabler(this)` in ShowModal) +// disables the main frame. That is directly observable: the WASM port re-emits a +// window's `enabled` flag into window.wxElementRegistry whenever DoEnable() runs +// (window.cpp UpdateElementRegistry), so the pcbnew main frame's registry entry +// flips enabled:true -> false the instant a real modal opens, and back to true on +// close. This is not a proxy for input-modality: the SAME wxWindow::IsEnabled() +// parent-walk that flips this flag is exactly what every input gate consults +// (app.cpp mouse/keyboard/wheel, domevents.cpp control/toolbar/menu-item events). +// enabled===false while a modal is open <=> the frame is input-modal. +// +// RED (bug present): PcbFrame.enabled stays `true` while the dialog is open +// (no wxWindowDisabler) -> assertion fails. +// GREEN (fixed): PcbFrame.enabled is `false` while open, `true` after close. +// +// IMPORTANT — dialog choice: this MUST drive a dialog shown via *real* +// wxDialog::ShowModal(). pcbnew's **Page Settings** dialog qualifies — +// board_editor_control.cpp:534 does `DIALOG_PAGES_SETTINGS dlg( ... ); +// dlg.ShowModal()`, and DIALOG_SHIM::ShowModal (dialog_shim.cpp:1386) forwards +// straight to wxDialog::ShowModal without adding a disabler of its own. +// +// Most other pcbnew dialogs are the WRONG target because KiCad already disables +// the parent for them, so they read "green" even unpatched: +// * Track & Via Properties AND the Plot dialog are both shown via +// ShowQuasiModal() (edit_tool.cpp:2306, board_editor_control.cpp:565), which +// runs KiCad's own nested loop and creates a WINDOW_DISABLER(parent) +// (dialog_shim.cpp:1431) — the parent frame is disabled with or without the +// wx fix. (Measured: opening Plot on the unpatched binary already reports +// PcbFrame.enabled === false. So quasi-modal dialogs do NOT exercise H-1.) +// H-1's real user impact in KiCad is therefore limited to the real-ShowModal +// dialogs, which is exactly what this test drives. + +// Open the File menu and click the menu item whose label matches `re` +// (menu labels carry a trailing ellipsis, so match by regex). Mirrors the proven +// pattern in plot-checklist.spec.ts. +async function clickFileMenuItem(page: Page, re: RegExp): Promise { + expect(await clickMenuBarItem(page, 'File'), 'File menu should open').toBe(true); + await page.waitForTimeout(600); + const items = await page.evaluate(() => { + const r = window.wxElementRegistry; + const all = (r?.findAllRendered?.({}) ?? []) as Array<{ + elementType: string; + label: string; + centerX: number; + centerY: number; + }>; + return all + .filter((e) => e.elementType === 'menuitem') + .map((e) => ({ label: e.label, x: e.centerX, y: e.centerY })); + }); + const target = items.find((i) => re.test(i.label)); + if (target) await page.mouse.click(target.x, target.y); + return items.map((i) => i.label); +} + +// The visible pcbnew main frame's registry entry (id + live enabled flag). +async function pcbFrame(page: Page): Promise<{ id: string; enabled: boolean } | null> { + return page.evaluate(() => { + const r = window.wxElementRegistry; + if (!r) return null; + const f = r.findAll({ visible: true }).find((e) => e.name === 'PcbFrame'); + return f ? { id: f.id, enabled: f.enabled } : null; + }); +} + +// The live `enabled` flag for a specific registry element id (the disable only +// re-emits the frame's own entry; children entries go stale — so read the frame). +async function enabledById(page: Page, id: string): Promise { + return page.evaluate((fid) => { + const el = window.wxElementRegistry?.getElement(fid); + return el ? el.enabled : null; + }, id); +} + +// Count currently-visible modal dialogs (KiCad dialog typeNames match /Dialog/i: +// e.g. "wxDialog", "wxFileDialog"). Used as an independent "a modal is open" +// signal that holds in both the RED and GREEN builds. +async function visibleDialogCount(page: Page): Promise { + return page.evaluate( + () => + window.wxElementRegistry + ?.findAll({ visible: true }) + .filter((e) => /Dialog/i.test(e.typeName)).length ?? 0, + ); +} + +test.describe('pcbnew modal dialogs are input-modal (H-1)', () => { + test.describe.configure({ timeout: 240000 }); + + test('opening a modal dialog disables the main pcbnew frame', async ({ page, testLogger }) => { + await page.goto('/kicad/pcbnew.html'); + await waitForPcbnew(page); + + // 1. Baseline: the main frame is registered and enabled, no modal open. + const before = await pcbFrame(page); + expect(before, 'PcbFrame should be registered').toBeTruthy(); + expect(before!.enabled, 'main frame is enabled before any modal opens').toBe(true); + const frameId = before!.id; + expect(await visibleDialogCount(page), 'no modal dialog open at baseline').toBe(0); + + // 2. Open the Page Settings dialog — a real wxDialog::ShowModal + // (board_editor_control.cpp:534). NOT Plot/Track&Via, which are quasi-modal. + const fileLabels = await clickFileMenuItem(page, /page settings/i); + console.log('[H1] File menu items: ' + JSON.stringify(fileLabels)); + + await expect + .poll(() => visibleDialogCount(page), { + timeout: 30000, + message: 'the Page Settings dialog (a real modal) should open', + }) + .toBeGreaterThan(0); + // The disabler is created synchronously inside ShowModal (after Show(true), + // before startModal() suspends), so the frame is already disabled by the time + // the modal is parked; a short settle keeps this robust. + await page.waitForTimeout(1000); + + await page.screenshot({ path: 'test-results/modal-input-lock-01-dialog.png', scale: 'device' }); + + // 3. PRIMARY: while the modal is open, the main pcbnew frame must be + // input-disabled. RED (no wxWindowDisabler) -> stays true -> FAILS here. + const duringModal = await enabledById(page, frameId); + console.log(`[H1] PcbFrame.enabled while the Page Settings modal is open: ${duringModal}`); + expect( + duringModal, + 'the main pcbnew frame must be disabled (input-modal) while a modal dialog is open — H-1', + ).toBe(false); + + // 4. Closing the modal re-enables the frame (proves the wxDELETE(m_windowDisabler) + // teardown path). Esc is KiCad's dialog-cancel. + await page.keyboard.press('Escape'); + await expect + .poll( + async () => { + if ((await visibleDialogCount(page)) > 0) await page.keyboard.press('Escape'); + return enabledById(page, frameId); + }, + { timeout: 20000, message: 'closing the modal should re-enable the main frame' }, + ) + .toBe(true); + + await page.screenshot({ path: 'test-results/modal-input-lock-02-closed.png', scale: 'device' }); + + // 5. The whole sequence must not trap the WASM module. + const aborted = [...testLogger.consoleLogs, ...testLogger.errors].some((l) => + l.includes('Aborted('), + ); + expect(aborted, 'the WASM module must not abort while toggling modal state').toBe(false); + }); +}); diff --git a/tests/playwright-kicad.config.ts b/tests/playwright-kicad.config.ts index 220311101..79a33fc0e 100644 --- a/tests/playwright-kicad.config.ts +++ b/tests/playwright-kicad.config.ts @@ -90,6 +90,8 @@ const BIG_MODULE_SPECS = [ // boots pcbnew.html — must run on V8 (chromium-ci); on Firefox/x86 CI the // ~190M module OOMs at instantiation and #canvas never appears (run 27626037849). "**/pcbnew-move.spec.ts", + // boots pcbnew.html and opens the Page Settings dialog (H-1 modal input-lock repro). + "**/modal-input-lock.spec.ts", // 3D viewer specs boot pcbnew.html (3D-enabled build) — same V8 routing. "**/3d-viewer.spec.ts", // Isolated (own file → own worker) so its heavy single load isn't degraded by the From 89a02dbb68d2106638498e922a5119ea461ce444 Mon Sep 17 00:00:00 2001 From: Istvan Matejcsok <119620946+matejcsok-ee@users.noreply.github.com> Date: Fri, 3 Jul 2026 12:22:34 +0200 Subject: [PATCH 03/14] =?UTF-8?q?test(kicad):=20H-5=20e2e=20=E2=80=94=20Pl?= =?UTF-8?q?ot=20dialog=20layer=20checks=20persist?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drives pcbnew's Plot dialog, whose layer list uses the exact Append-then-Check loop the bug breaks. RED <=1 layer checked / GREEN >=2 (the default plot layers). Bumps wxwidgets to the H-5 fix. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/kicad/plot-checklist.spec.ts | 109 +++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 tests/kicad/plot-checklist.spec.ts diff --git a/tests/kicad/plot-checklist.spec.ts b/tests/kicad/plot-checklist.spec.ts new file mode 100644 index 000000000..b1b598a10 --- /dev/null +++ b/tests/kicad/plot-checklist.spec.ts @@ -0,0 +1,109 @@ +import { test, expect } from './fixtures'; +import { clickMenuBarItem, clickByLabel } from '../e2e/utils/element-tracker'; +import type { Page } from '@playwright/test'; + +// KiCad-level reproduction of the wxCheckListBox "check marks wiped on rebuild" +// bug (parity audit H-5), fixed in src/wasm/{listbox.h,checklst.{h,cpp}}. +// +// pcbnew's Plot dialog fills its layer list with the exact Append-then-Check +// pattern the bug breaks (pcbnew/dialogs/dialog_plot.cpp:351-354): +// +// int checkIndex = m_layerCheckListBox->Append( board->GetLayerName( layer ) ); +// if( m_plotOpts.GetLayerSelection()[layer] ) +// m_layerCheckListBox->Check( checkIndex ); +// +// A fresh board's default plot selection is +// LSET{ F_SilkS, B_SilkS, F_Mask, B_Mask, F_Paste, B_Paste, Edge_Cuts } (+Cu), +// i.e. 7+ layers should be checked. With the DOM-port bug, every Append rebuilds +// the rows unchecked and only re-applies the listbox *selection*, never the +// *check* state — so all but the final Append's check are wiped and the dialog +// opens showing 0-1 checked layers. +// +// RED (bug present): <= 1 layer checkbox is checked in the Plot dialog. +// GREEN (fixed): the default plot layers (>= 2) are checked. + +async function waitForPcbnew(page: Page): Promise { + await expect(page.locator('#canvas')).toBeVisible({ timeout: 90000 }); + await page.waitForFunction(() => !!window.wxElementRegistry, null, { timeout: 90000 }); + await page.waitForFunction( + () => { + const r = window.wxElementRegistry; + return !!r && r.findAll({}).length > 0; + }, + null, + { timeout: 150000 }, + ); + await page.waitForTimeout(2500); + // dismiss the first-run setup wizard (Next > … Finish); no-op if absent + for (let i = 0; i < 12; i++) { + const advanced = await clickByLabel(page, 'Next >'); + if (!advanced) break; + await page.waitForTimeout(400); + } + await clickByLabel(page, 'Finish'); + await page.waitForTimeout(800); + await page.waitForFunction( + () => { + const r = window.wxElementRegistry; + if (!r) return false; + return r.findAll({ visible: true }).some((el) => el.name === 'PcbFrame'); + }, + null, + { timeout: 90000 }, + ); + await page.waitForTimeout(1500); +} + +// Open the File menu and click the menu item whose label matches `re`. +async function clickFileMenuItem(page: Page, re: RegExp): Promise { + expect(await clickMenuBarItem(page, 'File'), 'File menu should open').toBe(true); + await page.waitForTimeout(600); + const items = await page.evaluate(() => { + const r = window.wxElementRegistry; + const all = (r?.findAllRendered?.({}) ?? []) as any[]; + return all + .filter((e) => e.elementType === 'menuitem') + .map((e) => ({ label: e.label, x: e.centerX, y: e.centerY })); + }); + const labels = items.map((i) => i.label); + const target = items.find((i) => re.test(i.label)); + if (target) { + await page.mouse.click(target.x, target.y); + } + return labels; +} + +test.describe('pcbnew Plot dialog — wxCheckListBox check persistence (H-5)', () => { + test('Plot dialog shows the default plot layers checked', async ({ page }) => { + await page.goto('/kicad/pcbnew.html'); + await waitForPcbnew(page); + + const fileLabels = await clickFileMenuItem(page, /plot/i); + console.log('[PLOT] File menu items: ' + JSON.stringify(fileLabels)); + + // The Plot dialog's layer list is a wxCheckListBox -> a [data-wx-check-list] + // div of checkbox rows in the DOM port. + const boxes = page.locator('[data-wx-check-list] input[type=checkbox]'); + await expect + .poll(() => boxes.count(), { + timeout: 30000, + message: 'Plot dialog layer checklist should render its rows', + }) + .toBeGreaterThan(2); + + await page.screenshot({ path: 'test-results/plot-checklist.png', scale: 'device' }); + + const checked = await page + .locator('[data-wx-check-list] input[type=checkbox]:checked') + .count(); + const total = await boxes.count(); + console.log(`[PLOT] checked ${checked} of ${total} layer rows`); + + // RED: <= 1 checked (only the last Append survived). GREEN: the default + // plot layers (>= 2) are checked. + expect( + checked, + `expected the default plot layers to be checked (got ${checked}/${total})`, + ).toBeGreaterThanOrEqual(2); + }); +}); From 4e8eec9d850194c3bd91fb840430022138cb099c Mon Sep 17 00:00:00 2001 From: Istvan Matejcsok <119620946+matejcsok-ee@users.noreply.github.com> Date: Fri, 3 Jul 2026 15:07:47 +0200 Subject: [PATCH 04/14] =?UTF-8?q?test(kicad):=20H-2=20e2e=20=E2=80=94=20gr?= =?UTF-8?q?id=20cell=20text=20clips=20at=20the=20cell=20edge,=20no=20bleed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Boots pcbnew, opens Board Setup > Net Classes, adds a netclass whose 70-char name is ~7x wider than the fixed 96px Name column, commits it (second +-click: WX_GRID::OnAddRow runs CommitPendingChanges first — keys typed into a DOM editable never reach wx), then counts dark pixels inside the empty Clearance cell next to it from a device-scale screenshot. RED (unpatched wx): 325 ink pixels — the name bleeds across the whole grid. GREEN (patched): 0, with the name-cell ink identical in both runs (413), so only the clipping changed. Net Classes is load-bearing: the Text Variables grid cannot repro H-2 because WX_GRID::SetupColumnAutosizer auto-fits its column to the committed content, masking the missing clip. Also routes the spec to chromium-ci via PCBNEW_FAMILY_SPECS (pcbnew.html OOMs Firefox/x86 CI). Bumps wxwidgets: fix(wasm): make rectangular DC clips (SetClippingRegion) apply (parity H-2). Co-Authored-By: Claude Fable 5 --- tests/kicad/grid-clip-bleed.spec.ts | 283 ++++++++++++++++++++++++++++ tests/playwright-kicad.config.ts | 2 + 2 files changed, 285 insertions(+) create mode 100644 tests/kicad/grid-clip-bleed.spec.ts diff --git a/tests/kicad/grid-clip-bleed.spec.ts b/tests/kicad/grid-clip-bleed.spec.ts new file mode 100644 index 000000000..b313851e9 --- /dev/null +++ b/tests/kicad/grid-clip-bleed.spec.ts @@ -0,0 +1,283 @@ +import { test, expect } from './fixtures'; +import { clickMenuBarItem, clickTreeItem, findTreeItem, findGridCell } from '../e2e/utils/element-tracker'; +import { waitForPcbnew } from './utils/pcbnew-ready'; +import type { Page } from '@playwright/test'; + +// KiCad-level reproduction of the "rectangular DC clip is a silent no-op" bug +// (parity audit H-2), fixed in wxwidgets/src/wasm/dc.cpp. +// +// wxWasmDCImpl::DoSetClippingRegion(x,y,w,h) (src/wasm/dc.cpp:292-304) calls +// the base wxDCImpl::DoSetClippingRegion and then feeds m_clipX1..m_clipY2 to +// the JS clipRect. But the modern base class stores the clip box only in the +// *private* m_devClipX1.. members — m_clipX1.. stay at their ctor value of 0 — +// so JS always receives clipRect(id, 0,0,0,0), which wx.js treats as +// "uninitialized" and resets the clip to the whole context. Every wxDC-level +// rectangular clip (wxDCClipper, SetClippingRegion(wxRect)) is silently +// ignored. (The wxRegion overload works; only the coordinate overload is +// broken.) +// +// KiCad surface: wxGrid cell text is clipped to its cell via exactly this path +// (wxGrid::DrawTextRectangle -> wxDCClipper, src/generic/grid.cpp:7336), and +// KiCad's WX_GRID disables cell overflow (common/widgets/wx_grid.cpp:214 +// SetDefaultCellOverflow(false)), so text longer than its column must be +// truncated at the cell edge — even when the neighboring cell is empty. +// wxGrid paints cells in descending order (grid.cpp:6497), so with the bug the +// long text is painted *after* its right-hand neighbor and the unclipped +// overflow survives the paint pass, visibly bleeding across the next column. +// +// The surface is Board Setup > Net Classes: its grid has fixed column widths +// (no WX_GRID::SetupColumnAutosizer, which on e.g. the Text Variables grid +// auto-fits the column to its content and would mask the bug), and a newly +// added netclass row has an EMPTY Clearance cell next to the Name cell. +// +// RED (bug present): dark text pixels ("ink") inside the empty Clearance +// cell — the Name text bleeds across the column edge. +// GREEN (fixed): the Clearance cell interior stays background-only; +// the Name text is clipped at its own cell edge. + +const PAYLOAD = 'W'.repeat(70); // ~700px of text vs a ~130px Name column + +interface CssRect { + x: number; + y: number; + width: number; + height: number; +} + +function insetRect(r: { screenX: number; screenY: number; width: number; height: number }, + by: number): CssRect { + return { + x: r.screenX + by, + y: r.screenY + by, + width: Math.max(1, r.width - 2 * by), + height: Math.max(1, r.height - 2 * by), + }; +} + +// Measure "ink" (dark, text-colored) pixels and mean luminance of a +// viewport-CSS-coordinate rect in a device-scale screenshot. Grid cell +// backgrounds are white/near-white; text is black — luminance < 128 cleanly +// separates them. The mean guards against measuring a row that is still +// selected (dark navy row highlight), which would fake ink. +async function measureCell( + page: Page, + png: Buffer, + rect: CssRect, +): Promise<{ ink: number; mean: number }> { + return page.evaluate( + async ({ b64, rect }) => { + const img = new Image(); + await new Promise((resolve, reject) => { + img.onload = () => resolve(); + img.onerror = () => reject(new Error('screenshot decode failed')); + img.src = 'data:image/png;base64,' + b64; + }); + const canvas = document.createElement('canvas'); + canvas.width = img.width; + canvas.height = img.height; + const ctx = canvas.getContext('2d')!; + ctx.drawImage(img, 0, 0); + // The screenshot is viewport-sized at device scale; derive the ratio + // instead of trusting devicePixelRatio so both scales work. + const scale = img.width / window.innerWidth; + const data = ctx.getImageData( + Math.round(rect.x * scale), + Math.round(rect.y * scale), + Math.max(1, Math.round(rect.width * scale)), + Math.max(1, Math.round(rect.height * scale)), + ).data; + let ink = 0; + let lumSum = 0; + const pixels = data.length / 4; + for (let i = 0; i < data.length; i += 4) { + const lum = 0.299 * data[i] + 0.587 * data[i + 1] + 0.114 * data[i + 2]; + lumSum += lum; + if (lum < 128) ink++; + } + return { ink, mean: lumSum / pixels }; + }, + { b64: png.toString('base64'), rect }, + ); +} + +// Open the File menu and click the menu item whose label matches `re`. +async function clickFileMenuItem(page: Page, re: RegExp): Promise { + expect(await clickMenuBarItem(page, 'File'), 'File menu should open').toBe(true); + await page.waitForTimeout(600); + const items = await page.evaluate(() => { + const r = window.wxElementRegistry; + const all = r?.findAllRendered?.({}) ?? []; + return all + .filter((e) => e.elementType === 'menuitem') + .map((e) => ({ label: e.label, x: e.centerX, y: e.centerY })); + }); + console.log('[H2] File menu items: ' + JSON.stringify(items.map((i) => i.label))); + const target = items.find((i) => re.test(i.label)); + if (!target) return false; + await page.mouse.click(target.x, target.y); + return true; +} + +async function pollCellLabel( + page: Page, + row: number, + col: number, + expected: string, + timeoutMs: number, +): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + const cell = await findGridCell(page, row, col); + if (cell && cell.label === expected) return true; + await page.waitForTimeout(500); + } + return false; +} + +test.describe('pcbnew Board Setup Net Classes grid — DC rect clip (H-2)', () => { + test.describe.configure({ timeout: 240000 }); + + test('long netclass name does not bleed into the empty Clearance cell', async ({ page, testLogger }) => { + await page.goto('/kicad/pcbnew.html'); + await waitForPcbnew(page); + + // --- File > Board Setup... --- + expect( + await clickFileMenuItem(page, /board setup/i), + 'File menu should contain Board Setup', + ).toBe(true); + + // Dialog is open once its treebook renders; all nodes are expanded on open + // (dialog_board_setup.cpp:265), so the Net Classes item is clickable. + await expect + .poll(async () => (await findTreeItem(page, 'Net Classes')) !== null, { + timeout: 30000, + message: 'Board Setup dialog tree should render the Net Classes item', + }) + .toBe(true); + await page.screenshot({ path: 'test-results/grid-clip-bleed-01-dialog.png', scale: 'device' }); + + expect(await clickTreeItem(page, 'Net Classes')).toBe(true); + + // The lazily-built PANEL_SETUP_NETCLASSES page holds two grids: the + // netclass grid on top, the assignment grid below — take the topmost. + await expect + .poll( + async () => + page.evaluate(() => { + const r = window.wxElementRegistry; + return (r?.findAll({ visible: true }) ?? []).filter((e) => e.typeName === 'wxGrid').length; + }), + { timeout: 30000, message: 'Net Classes grids should appear' }, + ) + .toBeGreaterThan(0); + await page.waitForTimeout(800); + await page.screenshot({ path: 'test-results/grid-clip-bleed-02-page.png', scale: 'device' }); + + const grid = await page.evaluate(() => { + const r = window.wxElementRegistry; + const grids = (r?.findAll({ visible: true }) ?? []) + .filter((e) => e.typeName === 'wxGrid') + .sort((a, b) => a.screenY - b.screenY); + const g = grids[0]; + return g + ? { screenX: g.screenX, screenY: g.screenY, width: g.width, height: g.height } + : null; + }); + expect(grid, 'netclass wxGrid should be in the registry').not.toBeNull(); + console.log(`[H2] netclass grid rect: (${grid!.screenX},${grid!.screenY}) ${grid!.width}x${grid!.height}`); + + // --- add a row --- + // The panel's "+" STD_BITMAP_BUTTON is a custom-painted wxPanel and never + // enters the element registry, so click it blind: it is the leftmost + // button in the strip directly below the grid's bottom-left corner + // (offsets verified against the registry grid rect on the Text Variables + // page, which uses the identical sizer layout). OnAddNetclassClick -> + // WX_GRID::OnAddRow inserts a row at the TOP (row 0) and auto-opens the + // cell editor on (0, Name); the new row's Clearance cell stays empty. + const addBtn = { x: grid!.screenX + 13, y: grid!.screenY + grid!.height + 16 }; + const editorFocused = () => + page + .waitForFunction( + () => { + const a = document.activeElement; + return !!a && (a.tagName === 'INPUT' || a.tagName === 'TEXTAREA'); + }, + null, + { timeout: 10000 }, + ) + .then(() => true) + .catch(() => false); + + await page.mouse.click(addBtn.x, addBtn.y); + const editorOpen = await editorFocused(); + if (!editorOpen) { + await page.screenshot({ path: 'test-results/grid-clip-bleed-03-no-editor.png', scale: 'device' }); + } + expect(editorOpen, 'add-row should open the (0, Name) cell editor').toBe(true); + + await page.keyboard.type(PAYLOAD, { delay: 15 }); + await page.screenshot({ path: 'test-results/grid-clip-bleed-03-typed.png', scale: 'device' }); + + // Commit the editor. Keys typed into a DOM editable are consumed by the + // browser and never reach wx (audit #43/#55), so Enter cannot commit. + // Click the "+" button again instead: WX_GRID::OnAddRow first calls + // CommitPendingChanges() (mouse events do reach wx), committing the + // payload row — which the new top insertion shifts down to ROW 1 — and + // moving the cursor/row-selection highlight to the fresh row 0. Row 1 is + // then unhighlighted and stable for pixel measurement. + await page.mouse.click(addBtn.x, addBtn.y); + const committed = await pollCellLabel(page, 1, 0, PAYLOAD, 15000); + expect(committed, 'Name cell (1,0) should hold the committed payload').toBe(true); + + // --- measure --- + await page.waitForTimeout(1000); // let the post-commit repaint settle + const cellName = await findGridCell(page, 1, 0); + const cellClearance = await findGridCell(page, 1, 1); + expect(cellName, 'gridcell (1,0) should be registered').not.toBeNull(); + expect(cellClearance, 'gridcell (1,1) should be registered').not.toBeNull(); + console.log( + `[H2] cell rects: name=(${cellName!.screenX},${cellName!.screenY},${cellName!.width}x${cellName!.height}) ` + + `clearance=(${cellClearance!.screenX},${cellClearance!.screenY},${cellClearance!.width}x${cellClearance!.height})`, + ); + + const png = await page.screenshot({ + path: 'test-results/grid-clip-bleed-04-committed.png', + scale: 'device', + }); + + // 4px inset keeps gridlines, the cell-cursor border, and clip-edge + // antialiasing out of both samples. + const nameStats = await measureCell(page, png, insetRect(cellName!, 4)); + const clearanceStats = await measureCell(page, png, insetRect(cellClearance!, 4)); + console.log( + `[H2] name cell: ink=${nameStats.ink} mean=${nameStats.mean.toFixed(1)}; ` + + `clearance cell: ink=${clearanceStats.ink} mean=${clearanceStats.mean.toFixed(1)}`, + ); + + // Harness guards: row 1 must not carry the dark row-selection highlight + // (would fake ink), and the payload really was painted in the Name cell + // (guards against a vacuous pass from a failed commit). + expect( + clearanceStats.mean, + 'Clearance cell background should be light — row still selected?', + ).toBeGreaterThan(128); + expect(nameStats.ink, 'Name cell should contain painted text').toBeGreaterThan(50); + + // RED: hundreds of ink pixels — the Name text bleeds across the empty + // Clearance cell. GREEN: background only. + expect( + clearanceStats.ink, + `empty Clearance cell should contain no text ink (got ${clearanceStats.ink} dark pixels — Name-cell bleed)`, + ).toBeLessThan(5); + + // --- teardown & stability --- + await page.keyboard.press('Escape'); // close Board Setup (Cancel) + await page.waitForTimeout(500); + + const aborted = [...testLogger.consoleLogs, ...testLogger.errors].some((l) => + l.includes('Aborted('), + ); + expect(aborted, 'WASM module should not abort').toBe(false); + }); +}); diff --git a/tests/playwright-kicad.config.ts b/tests/playwright-kicad.config.ts index 79a33fc0e..e8749a575 100644 --- a/tests/playwright-kicad.config.ts +++ b/tests/playwright-kicad.config.ts @@ -92,6 +92,8 @@ const BIG_MODULE_SPECS = [ "**/pcbnew-move.spec.ts", // boots pcbnew.html and opens the Page Settings dialog (H-1 modal input-lock repro). "**/modal-input-lock.spec.ts", + // boots pcbnew.html and opens Board Setup > Text Variables (H-2 DC-clip repro). + "**/grid-clip-bleed.spec.ts", // 3D viewer specs boot pcbnew.html (3D-enabled build) — same V8 routing. "**/3d-viewer.spec.ts", // Isolated (own file → own worker) so its heavy single load isn't degraded by the From 03341778fadb6a95edb5627e79b5d9b993956187 Mon Sep 17 00:00:00 2001 From: Istvan Matejcsok <119620946+matejcsok-ee@users.noreply.github.com> Date: Tue, 7 Jul 2026 09:58:07 +0200 Subject: [PATCH 05/14] =?UTF-8?q?test(kicad):=20H-3=20e2e=20=E2=80=94=20di?= =?UTF-8?q?alog=20select-on-open=20replaces=20field=20text?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps wxwidgets (live DOM caret/selection) + kicad (SelectAll guard widened to __WXWASM__) to the H-3 fixes. E2e: single-track board, Ctrl+A then 'e' opens Track Properties with the pre-filled width field selected; typing '5' without clicking first replaces the value. Measured RED "0.255" (insert at caret) -> GREEN "5" (replaces selection). Co-Authored-By: Claude Opus 4.8 --- kicad | 2 +- tests/kicad/dialog-select-on-open.spec.ts | 189 ++++++++++++++++++++++ 2 files changed, 190 insertions(+), 1 deletion(-) create mode 100644 tests/kicad/dialog-select-on-open.spec.ts diff --git a/kicad b/kicad index ca8877324..f3a2e27e8 160000 --- a/kicad +++ b/kicad @@ -1 +1 @@ -Subproject commit ca8877324ce78d1bc9cb32bf634f091e0e76fb29 +Subproject commit f3a2e27e8a488ee29a760f690e5a917167facbd1 diff --git a/tests/kicad/dialog-select-on-open.spec.ts b/tests/kicad/dialog-select-on-open.spec.ts new file mode 100644 index 000000000..ac208d9d0 --- /dev/null +++ b/tests/kicad/dialog-select-on-open.spec.ts @@ -0,0 +1,189 @@ +import { test, expect } from './fixtures'; +import { waitForPcbnew } from './utils/pcbnew-ready'; + +// KiCad-level reproduction of the wxWidgets DOM-port "caret/selection never +// reaches the DOM input" bug family (parity audit H-3/#30/#31), fixed in +// wxwidgets/src/wasm/textentry.cpp + include/wx/wasm/private/dom.h + +// build/wasm/wx-dom.js, plus a one-line KiCad gate widening. +// +// Native behavior: when a DIALOG_SHIM opens, its first paint runs +// SelectAllInTextCtrls() and then focuses the initial-focus control +// (common/dialog_shim.cpp:1343-1357) — so the pre-filled text of the focused +// field is selected and typing REPLACES the value ("type-to-replace"). +// +// In the WASM build this is double-broken: +// 1. The SelectAll() inside SelectAllInTextCtrls is gated +// `#if defined(__WXMAC__) || defined(__WXMSW__)` (dialog_shim.cpp:901) and +// the wasm build defines __WXWASM__/__WXUNIVERSAL__ — compiled out. +// 2. Even where SelectAll()/SetSelection() IS called, the wasm port only +// writes a C++-side cache (src/wasm/textentry.cpp:150-178) — nothing calls +// setSelectionRange on the real DOM (#30), and the accessors never +// read selectionStart/End back (H-3). +// So the DOM input has no selection, and typing INSERTS at the browser caret +// instead of replacing the field. +// +// Surface: pcbnew Track & Via Properties on a board with a single TRACK. With +// tracks in the selection the dialog sets initial focus on m_TrackWidthCtrl +// (dialog_track_via_properties.cpp:845-846), a plain wxTextCtrl pre-filled +// with the track width — and wxWindowWasm::SetFocus() does reach the DOM +// (window.cpp:967 wxDomFocus), so keystrokes land in the real without +// any click. +// +// RED (bug present): typing "5" into the just-opened dialog INSERTS into +// the pre-filled width (e.g. "50.25" / "0.255"). +// GREEN (fixed): typing "5" REPLACES the selected width -> value "5". + +const DOC_DIR = `/home/kicad/documents`; +const PCB_PATH = `${DOC_DIR}/selectonopen.kicad_pcb`; +const TRACK_WIDTH_MM = 0.25; + +// A minimal board with a single track segment of a distinctive width. Ctrl+A +// selects exactly the track, satisfying the tracks/vias-only condition of +// PCB_ACTIONS::properties and making m_tracks true (initial focus -> width). +const BOARD = `(kicad_pcb + (version 20241229) + (generator "pcbnew") + (generator_version "9.0") + (general (thickness 1.6)) + (paper "A4") + (layers + (0 "F.Cu" signal) + (2 "B.Cu" signal) + ) + (net 0 "") + (segment (start 100 100) (end 120 100) (width ${TRACK_WIDTH_MM}) (layer "F.Cu") (net 0) + (uuid "88888888-0000-0000-0000-000000000001")) +) +`; + +// Open the board directly through the embind bridge (present in pcbnew.wasm). +async function openBoard(page: import('@playwright/test').Page): Promise { + await page.evaluate( + ({ dir, path, content }) => { + const w = window as unknown as { + FS: { mkdirTree(p: string): void; writeFile(p: string, d: string): void }; + Module: { kicadOpenFile(p: string): unknown }; + }; + try { + w.FS.mkdirTree(dir); + } catch { + /* exists */ + } + w.FS.writeFile(path, content); + w.Module.kicadOpenFile(path); + }, + { dir: DOC_DIR, path: PCB_PATH, content: BOARD }, + ); +} + +// The bounding box of the visible GAL WebGL canvas (some glcanvas-* are hidden). +async function visibleGlCanvasBox(page: import('@playwright/test').Page) { + const id = await page.evaluate(() => { + const c = Array.from(document.querySelectorAll('[id^="glcanvas-"]')) + .map((el) => el as HTMLCanvasElement) + .find((el) => { + const rect = el.getBoundingClientRect(); + return window.getComputedStyle(el).display !== 'none' && rect.width > 0 && rect.height > 0; + }); + return c?.id ?? null; + }); + expect(id, 'a visible GL canvas').not.toBeNull(); + const box = await page.locator(`#${id}`).boundingBox(); + expect(box, 'GL canvas bounding box').not.toBeNull(); + return box!; +} + +test.describe('pcbnew dialog select-all-on-open — caret/selection DOM bridge (H-3)', () => { + test.describe.configure({ timeout: 240000 }); + + test('typing into the just-opened Track Properties replaces the width value', async ({ + page, + testLogger, + }) => { + await page.goto('/kicad/pcbnew.html'); + await waitForPcbnew(page); + + await openBoard(page); + await page.waitForTimeout(3000); + await page.screenshot({ path: 'test-results/select-on-open-00-loaded.png', scale: 'device' }); + + // Focus the GAL canvas with a real click on an empty spot, Select All grabs + // the only track, E opens Track & Via Properties (proven C-1 harness). + const glBox = await visibleGlCanvasBox(page); + await page.mouse.click( + Math.round(glBox.x + glBox.width * 0.5), + Math.round(glBox.y + glBox.height * 0.2), + ); + await page.waitForTimeout(400); + await page.keyboard.press('Control+a'); + await page.waitForTimeout(700); + await page.keyboard.press('e'); + + // Dialog open signal: a visible wxDialog in the registry. + await expect + .poll( + async () => + page.evaluate(() => { + const r = window.wxElementRegistry; + return (r?.findAll({ visible: true }) ?? []).some((e) => /Dialog/i.test(e.typeName)); + }), + { timeout: 30000, message: 'Track & Via Properties dialog should open' }, + ) + .toBe(true); + + // Precondition (separates harness failure from RED): initial focus really + // landed on the pre-filled track-width . Match by parseFloat, not + // string equality (unit-binder formatting may vary). + await expect + .poll( + async () => + page.evaluate((expected: number) => { + const a = document.activeElement as HTMLInputElement | null; + if (!a || a.tagName !== 'INPUT') return null; + if (Math.abs(parseFloat(a.value) - expected) > 0.02) return null; + if (!a.id) a.id = '__h3_width_input'; + return a.value; + }, TRACK_WIDTH_MM), + { + timeout: 20000, + message: `initial focus should land on the pre-filled width (≈${TRACK_WIDTH_MM})`, + }, + ) + .not.toBeNull(); + + const before = await page.evaluate( + () => (document.getElementById('__h3_width_input') as HTMLInputElement).value, + ); + console.log(`[H3] width field value at dialog open: "${before}"`); + await page.screenshot({ path: 'test-results/select-on-open-01-dialog.png', scale: 'device' }); + + // Type WITHOUT clicking — the dialog's open-time select-all must make this + // replace the whole value, exactly as on native. + await page.keyboard.type('5'); + await page.waitForTimeout(500); + + const after = await page.evaluate( + () => (document.getElementById('__h3_width_input') as HTMLInputElement).value, + ); + console.log(`[H3] width field value after typing "5": "${after}"`); + await page.screenshot({ path: 'test-results/select-on-open-02-typed.png', scale: 'device' }); + + const aborted = [...testLogger.consoleLogs, ...testLogger.errors].some((l) => + l.includes('Aborted('), + ); + expect(aborted, 'WASM module should not abort').toBe(false); + + // RED: "5" is inserted into the pre-filled value (e.g. "50.25" / "0.255"). + // GREEN: the selected pre-fill is replaced -> exactly "5". + expect( + after, + `typing must replace the selected width value (got "${after}" from pre-fill "${before}")`, + ).toBe('5'); + + // Teardown: Escape twice (first may revert the field edit, second cancels). + await page.keyboard.press('Escape'); + await page.waitForTimeout(300); + await page.keyboard.press('Escape'); + await page.waitForTimeout(500); + }); +}); From 7f6e9ab5131c28feb40872135bc9845587ac98e1 Mon Sep 17 00:00:00 2001 From: Istvan Matejcsok <119620946+matejcsok-ee@users.noreply.github.com> Date: Tue, 7 Jul 2026 09:58:57 +0200 Subject: [PATCH 06/14] =?UTF-8?q?test(kicad):=20H-6=20e2e=20=E2=80=94=20ca?= =?UTF-8?q?lculator=20cable-size=20slider=20recomputes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps wxwidgets to the H-6 wxSlider scroll-event fix. E2e drives the PCB Calculator -> Cable Size current-density slider (wired only to wxEVT_SCROLL_*) and asserts a recomputed output field. Measured RED 0 fields change -> GREEN Ampacity 2.35619 -> 9.42478. Co-Authored-By: Claude Opus 4.8 --- tests/kicad/calc-slider-scroll.spec.ts | 130 +++++++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 tests/kicad/calc-slider-scroll.spec.ts diff --git a/tests/kicad/calc-slider-scroll.spec.ts b/tests/kicad/calc-slider-scroll.spec.ts new file mode 100644 index 000000000..819934194 --- /dev/null +++ b/tests/kicad/calc-slider-scroll.spec.ts @@ -0,0 +1,130 @@ +import type { Page } from '@playwright/test'; +import { test, expect } from './fixtures'; +import { clickByLabel, clickTreeItem } from '../e2e/utils/element-tracker'; + +// KiCad-level reproduction of the wxWidgets DOM-port "wxSlider fires only +// wxEVT_SLIDER, never the wxEVT_SCROLL_* family" bug (parity audit H-6), fixed +// in wxwidgets/src/wasm/slider.cpp. +// +// Native wxSlider fires the wxEVT_SCROLL_* family (THUMBTRACK/CHANGED/…) in +// addition to the wxEVT_SLIDER command event. The unfixed DOM port fired only +// wxEVT_SLIDER, so handlers bound EXCLUSIVELY to the scroll family never run. +// +// Surface: the PCB Calculator's "Cable Size" panel. Its current-density slider +// m_slCurrentDensity is connected ONLY to wxEVT_SCROLL_* → onUpdateCurrentDensity +// (panel_cable_size_base.cpp:283-286), whose body recomputes the output text +// fields (Ampacity etc.): `m_amp_by_mm2 = m_slCurrentDensity->GetValue(); updateAll(...)` +// (panel_cable_size.cpp:199-204). The effect is a readable value change, +// not a canvas repaint — so it is deterministically assertable (unlike the +// colour picker, which does not open in WASM, and the Appearance opacity +// sliders, whose effect is canvas-pixel only). +// +// RED (bug present): moving the current-density slider fires no scroll event, +// so onUpdateCurrentDensity never runs and NO output field +// changes. +// GREEN (fixed): moving the slider recomputes Ampacity (and derived +// fields) → at least one output value changes. + +async function waitForRegistry(page: Page): Promise { + await page.waitForFunction(() => !!window.wxElementRegistry, null, { timeout: 90000 }); + await page.waitForTimeout(2000); +} + +async function waitForLabel(page: Page, label: string, timeoutMs: number): Promise { + try { + await page.waitForFunction( + (l) => { + const r = window.wxElementRegistry; + return !!(r && r.findByLabel && r.findByLabel(l, {}).length > 0); + }, + label, + { timeout: timeoutMs }, + ); + return true; + } catch { + return false; + } +} + +// Same first-run wizard dismissal as calculator.spec.ts. +async function completeFirstRunWizard(page: Page): Promise { + await expect(page.locator('#canvas')).toBeVisible({ timeout: 90000 }); + await waitForRegistry(page); + for (let i = 1; i <= 12; i++) { + if (await waitForLabel(page, 'Next >', 15000)) { + if (await clickByLabel(page, 'Next >')) { + await page.waitForTimeout(400); + continue; + } + } + if (await waitForLabel(page, 'Finish', 5000)) { + await clickByLabel(page, 'Finish'); + await page.waitForTimeout(400); + } + break; + } + await page.waitForTimeout(2500); +} + +// Values of every editable text currently in the document (excludes the +// range slider itself and checkboxes/radios). Between two snapshots only the +// slider is moved, so any difference is attributable to it. +async function textInputValues(page: Page): Promise { + return page.evaluate(() => + Array.from( + document.querySelectorAll('input:not([type=range]):not([type=checkbox]):not([type=radio])'), + ).map((el) => (el as HTMLInputElement).value), + ); +} + +test.describe('PCB Calculator Cable Size — wxSlider scroll-event family (H-6)', () => { + test.describe.configure({ timeout: 180000 }); + + test('moving the current-density slider recomputes an output field', async ({ page, testLogger }) => { + await page.goto('/kicad/calculator.html'); + await completeFirstRunWizard(page); + + expect(await clickTreeItem(page, 'Cable Size'), 'Cable Size panel should be selectable').toBe(true); + await page.waitForTimeout(1200); + await page.screenshot({ path: 'test-results/calc-slider-00-panel.png', scale: 'device' }); + + // The current-density slider is the only wxSlider () on + // the panel; default value 3, range 3..12. + const slider = page.locator('input[type=range]').first(); + await expect(slider).toBeVisible({ timeout: 15000 }); + + const before = await textInputValues(page); + console.log(`[H6] output values before: ${JSON.stringify(before)}`); + + // Drive it to the far end and fire the DOM input/change events the port + // translates into the wxSlider event(s). + await slider.evaluate((el: HTMLInputElement) => { + el.value = '12'; + el.dispatchEvent(new Event('input', { bubbles: true })); + el.dispatchEvent(new Event('change', { bubbles: true })); + }); + await page.waitForTimeout(1200); + + const after = await textInputValues(page); + console.log(`[H6] output values after: ${JSON.stringify(after)}`); + await page.screenshot({ path: 'test-results/calc-slider-01-moved.png', scale: 'device' }); + + const aborted = [...testLogger.consoleLogs, ...testLogger.errors].some((l) => + l.includes('Aborted('), + ); + expect(aborted, 'WASM module should not abort').toBe(false); + + // Precondition: the slider is wired and the field set is stable. + expect(before.length, 'the panel should expose output text fields').toBeGreaterThan(0); + expect(after.length, 'the field set should be stable across the move').toBe(before.length); + + // RED: no output changed (scroll event never fired). GREEN: Ampacity et al. + // recomputed. + const changedCount = before.filter((v, i) => v !== after[i]).length; + console.log(`[H6] output fields changed by the slider move: ${changedCount}`); + expect( + changedCount, + 'moving the current-density slider must recompute at least one output field', + ).toBeGreaterThan(0); + }); +}); From 68d8ea58071c4956ae77e82f64d9a2407a0f8071 Mon Sep 17 00:00:00 2001 From: Istvan Matejcsok <119620946+matejcsok-ee@users.noreply.github.com> Date: Tue, 7 Jul 2026 10:00:55 +0200 Subject: [PATCH 07/14] =?UTF-8?q?test(kicad):=20H-7=20e2e=20=E2=80=94=20Ed?= =?UTF-8?q?it=20menu=20tracks=20the=20undo=20stack?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps wxwidgets to the H-7 wxEVT_MENU_OPEN fix. E2e opens the Edit menu on a one-footprint board and reads the Undo item's enabled state: RED frozen `true` on an empty stack -> GREEN `false` empty, `true` after an undoable move. Routes menu-undo-stale (H-7) AND dialog-select-on-open (H-3) into PCBNEW_FAMILY_SPECS and syncs the H-2 routing comment (shared config plumbing). Co-Authored-By: Claude Opus 4.8 --- tests/kicad/menu-undo-stale.spec.ts | 182 ++++++++++++++++++++++++++++ tests/playwright-kicad.config.ts | 6 +- 2 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 tests/kicad/menu-undo-stale.spec.ts diff --git a/tests/kicad/menu-undo-stale.spec.ts b/tests/kicad/menu-undo-stale.spec.ts new file mode 100644 index 000000000..480632fd2 --- /dev/null +++ b/tests/kicad/menu-undo-stale.spec.ts @@ -0,0 +1,182 @@ +import { test, expect } from './fixtures'; +import { clickMenuBarItem } from '../e2e/utils/element-tracker'; +import type { Page } from '@playwright/test'; + +// KiCad-level reproduction of the wxWidgets DOM-port "menu enable/check goes +// stale" bug (parity audit H-7), fixed in wxwidgets/src/wasm/menu.cpp + +// domevents.cpp + include/wx/wasm/{window,menu}.h + build/wasm/wx-dom.js. +// +// Native behavior: opening a menu fires wxEVT_MENU_OPEN, and KiCad refreshes +// each item's enable/check state just-in-time (ACTION_MENU::OnMenuEvent runs +// ACTIONS::updateMenu, gated on wxEVT_MENU_OPEN — action_menu.cpp:421-426). +// +// In the WASM/DOM port that event is NEVER fired: clicking a menubar title +// opens the popup from a cached JS snapshot (build/wasm/wx-dom.js) and never +// calls back into C++. The menu is only re-serialized to the DOM on structural +// mutations (Append/Insert/Remove) — so every item keeps the enable/check +// state it had at construction/attach time. wxUSE_IDLEMENUUPDATES==1 refreshes +// the C++ item state on idle, but nothing pushes it to the DOM. KiCad's entire +// menu enable/check refresh is therefore dead. +// +// Surface: Edit ▸ Undo. Undo is disabled while the undo stack is empty and must +// enable after an undoable action. We load a board (empty undo stack), read the +// Undo item's enabled flag, perform one real BOARD_COMMIT move via the embind +// hook (pushes one undo entry), then re-read. +// +// RED (bug present): the Undo item is frozen at its construction default +// (enabled=true) and never tracks the undo stack — so it +// reads enabled=true even with an empty stack, and does +// not change after the action. +// GREEN (fixed): Undo reads enabled=false with an empty stack and +// enabled=true after the move — it tracks the stack. + +const DOC_DIR = `/home/kicad/documents`; +const PCB_PATH = `${DOC_DIR}/menu_undo.kicad_pcb`; + +// A minimal board with one footprint (the item kicadCollabTestMoveFirst moves, +// proven in save-hook.spec.ts / pcbnew-collab.spec.ts). The move runs through a +// BOARD_COMMIT::Push → one undo entry. Loading a board does not push undo +// entries, so the undo stack is empty until the move. +const BOARD = `(kicad_pcb + (version 20241229) + (generator "pcbnew") + (generator_version "9.0") + (general (thickness 1.6)) + (paper "A4") + (layers + (0 "F.Cu" signal) + (2 "B.Cu" signal) + (37 "F.SilkS" user) + (25 "Edge.Cuts" user) + ) + (setup) + (net 0 "") + (footprint "TestLib:R" + (layer "F.Cu") + (uuid "99999999-0000-0000-0000-000000000001") + (at 100 100) + (attr smd) + (property "Reference" "R1" (at 0 -4.2 0) (layer "F.SilkS") (uuid "99999999-0000-0000-0000-0000000000aa") (effects (font (size 1 1) (thickness 0.15)))) + ) +) +`; + +// Boot the seeded pcbnew-collab harness (skips the first-run wizard, and is the +// proven context for kicadCollabTestMoveFirst — save-hook/pcbnew-collab specs). +async function bootPcbnew(page: Page): Promise { + await expect(page.locator('#canvas')).toBeVisible({ timeout: 90000 }); + await page.waitForFunction(() => !!window.wxElementRegistry, null, { timeout: 90000 }); + await page.waitForFunction( + () => { + const m = (window as unknown as { Module?: Record }).Module; + return ( + typeof m?.kicadOpenFile === 'function' && + typeof m?.kicadCollabTestMoveFirst === 'function' && + typeof m?.kicadCollabGetPos === 'function' + ); + }, + null, + { timeout: 90000 }, + ); + await page.waitForFunction( + () => + !!window.wxElementRegistry && + window.wxElementRegistry + .findAll({ visible: true }) + .some((e) => /Frame$/.test(e.typeName) || (e.name || '').endsWith('Frame')), + null, + { timeout: 90000 }, + ); +} + +// A click on the GAL canvas pumps the wasm main loop so deferred work +// (kicadCollabTestMoveFirst queues its BOARD_COMMIT via CallAfter+coroutine) +// actually drains — see save-hook.spec.ts's focusCanvas. +async function pumpCanvas(page: Page): Promise { + const box = await page.locator('#canvas').boundingBox(); + if (box) await page.mouse.click(box.x + box.width / 2, box.y + box.height / 2); + await page.waitForTimeout(400); +} + +async function openBoard(page: Page): Promise { + await page.evaluate( + ({ dir, path, content }) => { + const w = window as unknown as { + FS: { mkdirTree(p: string): void; writeFile(p: string, d: string): void }; + Module: { kicadOpenFile(p: string): unknown }; + }; + try { + w.FS.mkdirTree(dir); + } catch { + /* exists */ + } + w.FS.writeFile(path, content); + w.Module.kicadOpenFile(path); + }, + { dir: DOC_DIR, path: PCB_PATH, content: BOARD }, + ); +} + +// Open the Edit menu, read the Undo item's enabled flag from the rendered +// registry, then close the menu again. +async function readUndoEnabled(page: Page): Promise { + expect(await clickMenuBarItem(page, 'Edit'), 'Edit menu should open').toBe(true); + await page.waitForTimeout(700); + const enabled = await page.evaluate(() => { + const r = window.wxElementRegistry; + const items = (r?.findAllRendered?.({}) ?? []).filter((e) => e.elementType === 'menuitem'); + const undo = items.find((e) => /^Undo\b/.test(e.label || '')); + return undo ? undo.enabled : null; + }); + await page.keyboard.press('Escape'); + await page.waitForTimeout(400); + return enabled; +} + +test.describe('pcbnew Edit menu — Undo enable tracks the undo stack (H-7)', () => { + test.describe.configure({ timeout: 240000 }); + + test('Undo enable state refreshes when the menu opens', async ({ page, testLogger }) => { + await page.goto('/kicad/pcbnew-collab.html'); + await bootPcbnew(page); + + await openBoard(page); + await page.waitForTimeout(3000); + await page.screenshot({ path: 'test-results/menu-undo-00-loaded.png', scale: 'device' }); + + // --- baseline: empty undo stack --- + const E0 = await readUndoEnabled(page); + console.log(`[H7] Undo.enabled with an empty undo stack: ${E0}`); + expect(E0, 'Undo menu item should be registered').not.toBeNull(); + + // --- undoable action: move the first item via a real BOARD_COMMIT --- + const movedId = await page.evaluate(() => + (window as unknown as { Module: { kicadCollabTestMoveFirst(dx: number, dy: number): string } }) + .Module.kicadCollabTestMoveFirst(2_000_000, 0), + ); + expect(movedId, 'an item should have been picked to move').toBeTruthy(); + + // The move is deferred (CallAfter + coroutine); a canvas click pumps the + // wasm main loop so the BOARD_COMMIT::Push actually runs (dirtying the + // board → one undo entry, as save-hook.spec.ts relies on). Pump a few times + // to be sure the deferred work drained. + for (let i = 0; i < 3; i++) await pumpCanvas(page); + console.log(`[H7] moved item ${movedId}; board dirtied via BOARD_COMMIT`); + + // --- post-action --- + const E1 = await readUndoEnabled(page); + console.log(`[H7] Undo.enabled after an undoable move: ${E1}`); + await page.screenshot({ path: 'test-results/menu-undo-01-after.png', scale: 'device' }); + + const aborted = [...testLogger.consoleLogs, ...testLogger.errors].some((l) => + l.includes('Aborted('), + ); + expect(aborted, 'WASM module should not abort').toBe(false); + + // RED trap: the frozen construction default is enabled=true, so an empty + // undo stack must read false only when the menu re-serializes on open. + expect(E0, 'Undo should be DISABLED with an empty undo stack').toBe(false); + expect(E1, 'Undo should be ENABLED after an undoable action').toBe(true); + expect(E1, 'Undo enable state must change with the undo stack').not.toBe(E0); + }); +}); diff --git a/tests/playwright-kicad.config.ts b/tests/playwright-kicad.config.ts index e8749a575..d1fb80661 100644 --- a/tests/playwright-kicad.config.ts +++ b/tests/playwright-kicad.config.ts @@ -92,8 +92,12 @@ const BIG_MODULE_SPECS = [ "**/pcbnew-move.spec.ts", // boots pcbnew.html and opens the Page Settings dialog (H-1 modal input-lock repro). "**/modal-input-lock.spec.ts", - // boots pcbnew.html and opens Board Setup > Text Variables (H-2 DC-clip repro). + // boots pcbnew.html and opens Board Setup > Net Classes (H-2 DC-clip repro). "**/grid-clip-bleed.spec.ts", + // boots pcbnew.html and opens Track & Via Properties (H-3 select-on-open repro). + "**/dialog-select-on-open.spec.ts", + // boots pcbnew.html and opens the Edit menu (H-7 menu-staleness repro). + "**/menu-undo-stale.spec.ts", // 3D viewer specs boot pcbnew.html (3D-enabled build) — same V8 routing. "**/3d-viewer.spec.ts", // Isolated (own file → own worker) so its heavy single load isn't degraded by the From e8f3b4cbdc65d77eb95db3ee674604d65b50ffc4 Mon Sep 17 00:00:00 2001 From: Istvan Matejcsok <119620946+matejcsok-ee@users.noreply.github.com> Date: Tue, 7 Jul 2026 10:03:04 +0200 Subject: [PATCH 08/14] test(standalone): wxWidgets DOM-port vs native parity-audit reproductions Standalone Emscripten test apps (one per finding) + their Makefile.wasm targets and the Playwright parity-audit spec, each driving the exact buggy path and self-reporting RED/GREEN: selevent-clientdata (C-1) textctrl-clear (H-4) checklist-checks (H-5) slider-scroll (H-6) config-utf8 (H-9) stattext-mnemonic (#36) textsel (H-3 caret/selection read+write) Bumps wxwidgets past the H-4/H-9/#36 fixes exercised here (H-3/H-6 already bumped by their KiCad e2e commits; C-1/H-5 fixes were committed earlier). Co-Authored-By: Claude Opus 4.8 --- tests/apps/Makefile.wasm | 81 +++++- .../checklist-checks_test.cpp | 77 +++++ .../config-utf8/config-utf8_test.cpp | 120 ++++++++ .../selevent-clientdata_test.cpp | 137 +++++++++ .../slider-scroll/slider-scroll_test.cpp | 88 ++++++ .../stattext-mnemonic_test.cpp | 57 ++++ .../textctrl-clear/textctrl-clear_test.cpp | 81 ++++++ .../apps/standalone/textsel/textsel_test.cpp | 96 +++++++ tests/e2e/parity-audit.spec.ts | 269 ++++++++++++++++++ 9 files changed, 1004 insertions(+), 2 deletions(-) create mode 100644 tests/apps/standalone/checklist-checks/checklist-checks_test.cpp create mode 100644 tests/apps/standalone/config-utf8/config-utf8_test.cpp create mode 100644 tests/apps/standalone/selevent-clientdata/selevent-clientdata_test.cpp create mode 100644 tests/apps/standalone/slider-scroll/slider-scroll_test.cpp create mode 100644 tests/apps/standalone/stattext-mnemonic/stattext-mnemonic_test.cpp create mode 100644 tests/apps/standalone/textctrl-clear/textctrl-clear_test.cpp create mode 100644 tests/apps/standalone/textsel/textsel_test.cpp create mode 100644 tests/e2e/parity-audit.spec.ts diff --git a/tests/apps/Makefile.wasm b/tests/apps/Makefile.wasm index 324d2e4d3..9aec49888 100644 --- a/tests/apps/Makefile.wasm +++ b/tests/apps/Makefile.wasm @@ -65,7 +65,7 @@ CXXFLAGS += $(EH_FLAGS) # - js_writeTextToClipboard, js_readTextFromClipboard, js_clipboardHasText, js_clearClipboard: for clipboard # - js_enumerateFonts: for font enumeration via Local Font Access API BASE_LDFLAGS = $(EH_FLAGS) -sALLOW_MEMORY_GROWTH -sERROR_ON_UNDEFINED_SYMBOLS=0 \ - -s "EXPORTED_RUNTIME_METHODS=['HEAPU8','HEAP8','HEAP32','ccall']" \ + -s "EXPORTED_RUNTIME_METHODS=['HEAPU8','HEAP8','HEAP32','ccall','stringToNewUTF8']" \ -sASYNCIFY=1 \ -sASYNCIFY_STACK_SIZE=65536 \ -sASYNCIFY_IMPORTS=['startModal','js_writeTextToClipboard','js_readTextFromClipboard','js_clipboardHasText','js_clearClipboard','js_enumerateFonts'] @@ -213,7 +213,14 @@ all: minimal_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 + $(S)/warp-pointer/warp-pointer_test.html \ + $(S)/selevent-clientdata/selevent-clientdata_test.html \ + $(S)/textctrl-clear/textctrl-clear_test.html \ + $(S)/checklist-checks/checklist-checks_test.html \ + $(S)/slider-scroll/slider-scroll_test.html \ + $(S)/config-utf8/config-utf8_test.html \ + $(S)/stattext-mnemonic/stattext-mnemonic_test.html \ + $(S)/textsel/textsel_test.html # Main test app minimal_test.o: minimal_test.cpp @@ -270,6 +277,76 @@ $(S)/warp-pointer/warp-pointer_test.html: $(S)/warp-pointer/warp-pointer_test.o warp-pointer: $(S)/warp-pointer/warp-pointer_test.html +# --- wxWidgets DOM-port vs native parity reproductions (parity-audit) --------- + +# Selection command events must carry per-item client data (choice/listbox/ +# combobox) — else KiCad's Track & Via Properties dialog null-derefs. +$(S)/selevent-clientdata/selevent-clientdata_test.o: $(S)/selevent-clientdata/selevent-clientdata_test.cpp + $(CXX) -c $(CXXFLAGS) $< -o $@ + +$(S)/selevent-clientdata/selevent-clientdata_test.html: $(S)/selevent-clientdata/selevent-clientdata_test.o $(WX_CORE_LIB) $(JS_FILES) + $(CXX) $< $(LDFLAGS_NOGL) --pre-js $(JS) --shell-file $(HTML) -o $@ + +selevent-clientdata: $(S)/selevent-clientdata/selevent-clientdata_test.html + +# wxTextCtrl::Clear()/Remove() must update the visible element. +$(S)/textctrl-clear/textctrl-clear_test.o: $(S)/textctrl-clear/textctrl-clear_test.cpp + $(CXX) -c $(CXXFLAGS) $< -o $@ + +$(S)/textctrl-clear/textctrl-clear_test.html: $(S)/textctrl-clear/textctrl-clear_test.o $(WX_CORE_LIB) $(JS_FILES) + $(CXX) $< $(LDFLAGS_NOGL) --pre-js $(JS) --shell-file $(HTML) -o $@ + +textctrl-clear: $(S)/textctrl-clear/textctrl-clear_test.html + +# wxCheckListBox check marks must survive item-list rebuilds. +$(S)/checklist-checks/checklist-checks_test.o: $(S)/checklist-checks/checklist-checks_test.cpp + $(CXX) -c $(CXXFLAGS) $< -o $@ + +$(S)/checklist-checks/checklist-checks_test.html: $(S)/checklist-checks/checklist-checks_test.o $(WX_CORE_LIB) $(JS_FILES) + $(CXX) $< $(LDFLAGS_NOGL) --pre-js $(JS) --shell-file $(HTML) -o $@ + +checklist-checks: $(S)/checklist-checks/checklist-checks_test.html + +# wxSlider must fire the wxEVT_SCROLL_* family, not only wxEVT_SLIDER. +$(S)/slider-scroll/slider-scroll_test.o: $(S)/slider-scroll/slider-scroll_test.cpp + $(CXX) -c $(CXXFLAGS) $< -o $@ + +$(S)/slider-scroll/slider-scroll_test.html: $(S)/slider-scroll/slider-scroll_test.o $(WX_CORE_LIB) $(JS_FILES) + $(CXX) $< $(LDFLAGS_NOGL) --pre-js $(JS) --shell-file $(HTML) -o $@ + +slider-scroll: $(S)/slider-scroll/slider-scroll_test.html + +# wxConfig must round-trip non-ASCII strings without truncation. +$(S)/config-utf8/config-utf8_test.o: $(S)/config-utf8/config-utf8_test.cpp + $(CXX) -c $(CXXFLAGS) $< -o $@ + +$(S)/config-utf8/config-utf8_test.html: $(S)/config-utf8/config-utf8_test.o $(WX_CORE_LIB) $(JS_FILES) + $(CXX) $< $(LDFLAGS_NOGL) --pre-js $(JS) --shell-file $(HTML) -o $@ + +config-utf8: $(S)/config-utf8/config-utf8_test.html + +# wxStaticText must consume the mnemonic '&' (KiCad dialog labels show a stray &). +$(S)/stattext-mnemonic/stattext-mnemonic_test.o: $(S)/stattext-mnemonic/stattext-mnemonic_test.cpp + $(CXX) -c $(CXXFLAGS) $< -o $@ + +$(S)/stattext-mnemonic/stattext-mnemonic_test.html: $(S)/stattext-mnemonic/stattext-mnemonic_test.o $(WX_CORE_LIB) $(JS_FILES) + $(CXX) $< $(LDFLAGS_NOGL) --pre-js $(JS) --shell-file $(HTML) -o $@ + +stattext-mnemonic: $(S)/stattext-mnemonic/stattext-mnemonic_test.html + +# wxTextEntry caret/selection must live-read/write the DOM input (H-3/#30). +$(S)/textsel/textsel_test.o: $(S)/textsel/textsel_test.cpp + $(CXX) -c $(CXXFLAGS) $< -o $@ + +$(S)/textsel/textsel_test.html: $(S)/textsel/textsel_test.o $(WX_CORE_LIB) $(JS_FILES) + $(CXX) $< $(LDFLAGS_NOGL) --pre-js $(JS) --shell-file $(HTML) -o $@ + +textsel: $(S)/textsel/textsel_test.html + +# Aggregate: build every parity-audit reproduction in one go. +parity-repros: selevent-clientdata textctrl-clear checklist-checks slider-scroll config-utf8 stattext-mnemonic textsel +.PHONY: parity-repros selevent-clientdata textctrl-clear checklist-checks slider-scroll config-utf8 stattext-mnemonic textsel + # Menu test (no GL) $(S)/menu/menu_test.o: $(S)/menu/menu_test.cpp $(CXX) -c $(CXXFLAGS) $< -o $@ diff --git a/tests/apps/standalone/checklist-checks/checklist-checks_test.cpp b/tests/apps/standalone/checklist-checks/checklist-checks_test.cpp new file mode 100644 index 000000000..1541cc6f2 --- /dev/null +++ b/tests/apps/standalone/checklist-checks/checklist-checks_test.cpp @@ -0,0 +1,77 @@ +// wxCheckListBox check marks must survive item-list rebuilds (DOM port). +// +// Bug (src/wasm/listbox.cpp + checklst.cpp): +// Any Append/Insert/Delete/SetString on the (check)listbox calls +// wxListBox::WasmSyncItems(), which rebuilds every DOM row UNCHECKED +// (wxDomSetItems) and then re-applies only m_itemsSelected via +// WasmSyncSelection(). wxCheckListBox keeps its check state in a SEPARATE +// array m_itemsChecked that is never re-pushed — and the DOM checklist has a +// single boolean per row (wxDomSetItemSelected drives the row checkbox). So +// every Check() done before a later Append() is wiped from the DOM. +// +// The canonical KiCad pattern is Append-then-Check in a loop: +// int i = clb->Append(name); +// if (enabled) clb->Check(i); +// (dialog_plot.cpp, dialog_print_*.cpp, ...). Each Append rebuilds the rows +// unchecked, so all but the final Append's check vanish — the Plot/Print +// layer checklists show the wrong checkboxes even though the C++ cache is fine. +// +// This app appends 5 rows and checks the even indices (0, 2, 4) with that exact +// Append-then-Check pattern. The spec reads the live DOM checkboxes: +// RED (bug present): only the last-checked row (4) is checked in the DOM. +// GREEN (fixed): rows 0, 2, 4 are checked; rows 1, 3 are not. + +#include "wx/wxprec.h" +#ifndef WX_PRECOMP + #include "wx/wx.h" +#endif + +#include "wx/checklst.h" + +#ifdef __EMSCRIPTEN__ +#include +#endif + +class ReproFrame : public wxFrame +{ +public: + ReproFrame(); +}; + +ReproFrame::ReproFrame() + : wxFrame(nullptr, wxID_ANY, "wxCheckListBox checks repro", + wxDefaultPosition, wxSize(320, 280)) +{ + wxBoxSizer *sizer = new wxBoxSizer(wxVERTICAL); + + wxCheckListBox *clb = new wxCheckListBox(this, wxID_ANY); + sizer->Add(clb, 1, wxALL | wxEXPAND, 10); + + // Append-then-Check loop, exactly like KiCad's layer checklists. + for (int i = 0; i < 5; i++) + { + const int n = clb->Append(wxString::Format("Layer %d", i)); + if (i % 2 == 0) // check the even rows: 0, 2, 4 + clb->Check(n, true); + } + + SetSizer(sizer); + +#ifdef __EMSCRIPTEN__ + CallAfter([] { EM_ASM({ console.log('[REPRO] checklist 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/apps/standalone/config-utf8/config-utf8_test.cpp b/tests/apps/standalone/config-utf8/config-utf8_test.cpp new file mode 100644 index 000000000..a0ede3a84 --- /dev/null +++ b/tests/apps/standalone/config-utf8/config-utf8_test.cpp @@ -0,0 +1,120 @@ +// wxConfig must round-trip non-ASCII strings without truncation (DOM port). +// +// Bug (src/wasm/config.cpp + build/wasm/wx.js): +// DoReadString() sizes its C++ buffer from getConfigEntryLength(), and the +// enumeration paths (GetNextEntry/GetNextGroup) from getConfigKeyLength(). +// Both JS helpers return the JavaScript string's `.length` — the number of +// UTF-16 code units — but the C++ side feeds that to stringToUTF8() as a max +// BYTE budget. For any non-ASCII text the UTF-8 byte length exceeds the +// UTF-16 unit count, so the buffer is undersized and the value/name is +// truncated at the first multi-byte character (writes are fine; only the +// read-back is corrupted). A "café" written and read back returns "caf". +// +// This silently corrupts recent-file paths and any wxConfig-backed string for +// i18n users (e.g. /home/José/board.kicad_pcb). +// +// This app writes a non-ASCII VALUE and a non-ASCII entry NAME, reads them back +// in the same session, and self-reports: +// RED (bug present): the read-back is truncated -> [REPRO] ...: FAIL +// GREEN (fixed): the read-back equals what was written -> PASS + +#include "wx/wxprec.h" +#ifndef WX_PRECOMP + #include "wx/wx.h" +#endif + +#include "wx/config.h" +#include "wx/wasm/config.h" + +#ifdef __EMSCRIPTEN__ +#include +#endif + +static void Repro(const wxString& line) +{ +#ifdef __EMSCRIPTEN__ + EM_ASM({ console.log('[REPRO] ' + UTF8ToString($0)); }, + (const char *)line.utf8_str()); +#endif +} + +static void RunConfigTest() +{ + wxLocalStorageConfig cfg("wxUtf8ReproTest"); + + // start from a clean group so a previous run can't pollute enumeration + cfg.DeleteGroup("/reprotest"); + + // ---- 1) non-ASCII VALUE round-trip ------------------------------------- + const wxString value = + wxString::FromUTF8("café-résumé-naïve-Žluťoučký-日本語-Ω"); + + cfg.Write("/reprotest/path", value); + cfg.Flush(); + + wxString readValue; + const bool readOk = cfg.Read("/reprotest/path", &readValue); + if (readOk && readValue == value) + Repro("config_value: PASS"); + else + Repro("config_value: FAIL (read back '" + readValue + "')"); + + // ---- 2) non-ASCII entry NAME round-trip (enumeration) ------------------ + // Write with an ABSOLUTE key so the stored localStorage key is well-formed + // (a relative write under a non-root path is a separate, unrelated bug), and + // the only thing under test is getConfigKeyLength's byte budget. + const wxString keyName = wxString::FromUTF8("náme_日_key"); + + cfg.Write("/reprotest/" + keyName, "x"); + cfg.Flush(); + + bool found = false; + cfg.SetPath("/reprotest"); + wxString entry; + long idx = 0; + if (cfg.GetFirstEntry(entry, idx)) + { + do + { + if (entry == keyName) + found = true; + } while (cfg.GetNextEntry(entry, idx)); + } + Repro(found ? "config_keyname: PASS" + : "config_keyname: FAIL (non-ASCII entry name truncated)"); + + cfg.DeleteGroup("/reprotest"); +} + +class ReproFrame : public wxFrame +{ +public: + ReproFrame() + : wxFrame(nullptr, wxID_ANY, "wxConfig UTF-8 repro", + wxDefaultPosition, wxSize(320, 120)) + { + new wxStaticText(this, wxID_ANY, "wxConfig UTF-8 round-trip test", + wxPoint(10, 10)); + +#ifdef __EMSCRIPTEN__ + CallAfter([] { + RunConfigTest(); + EM_ASM({ console.log('[REPRO] config 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/apps/standalone/selevent-clientdata/selevent-clientdata_test.cpp b/tests/apps/standalone/selevent-clientdata/selevent-clientdata_test.cpp new file mode 100644 index 000000000..1f0956052 --- /dev/null +++ b/tests/apps/standalone/selevent-clientdata/selevent-clientdata_test.cpp @@ -0,0 +1,137 @@ +// Selection command events must carry per-item client data (DOM port). +// +// Bug (src/wasm/{choice,listbox,combobox}.cpp, OnDomEvent): +// +// wxCommandEvent event(wxEVT_CHOICE, GetId()); +// event.SetInt(m_selection); +// event.SetString(GetString(m_selection)); +// event.SetEventObject(this); +// HandleWindowEvent(event); // <-- no client data EVER attached +// +// The hand-rolled wxEVT_CHOICE / wxEVT_LISTBOX / wxEVT_COMBOBOX events never call +// InitCommandEventWithItems(), so event.GetClientData()/GetClientObject() always +// return NULL even when the picked item was appended WITH client data. Native +// ports route through wxControlWithItemsBase::SendSelectionChangedEvent(), which +// copies the selected item's client object/data into the event +// (src/common/ctrlsub.cpp). +// +// This is a real crash in KiCad: pcbnew's Track & Via Properties dialog does +// static_cast(aEvent.GetClientData())->m_Diameter +// with no null guard (dialog_track_via_properties.cpp onViaSelect) — a normal +// "pick a predefined via size" action then traps the WASM module. +// +// The repro appends items WITH typed client data (wxStringClientData), then the +// spec fires a real DOM 'change' on each control's element. The bound handler +// reads the event's client object and self-reports: +// +// RED (bug present): GetClientObject() == NULL -> [REPRO] : FAIL +// GREEN (fixed): GetClientObject() == the picked item's data -> PASS + +#include "wx/wxprec.h" +#ifndef WX_PRECOMP + #include "wx/wx.h" +#endif + +#include "wx/combobox.h" + +#ifdef __EMSCRIPTEN__ +#include +#endif + +static void Repro(const wxString& line) +{ +#ifdef __EMSCRIPTEN__ + EM_ASM({ console.log('[REPRO] ' + UTF8ToString($0)); }, + (const char *)line.utf8_str()); +#else + wxPrintf("[REPRO] %s\n", line); +#endif +} + +// Expected client-data payload for item index n: "DATA_A", "DATA_B", ... +static wxString ExpectedData(int n) +{ + return wxString::Format("DATA_%c", (char)('A' + n)); +} + +// Read the selection event's client OBJECT and report PASS/FAIL for `name`. +static void CheckEvent(const wxString& name, wxCommandEvent& evt) +{ + const int sel = evt.GetInt(); + wxClientData *obj = evt.GetClientObject(); + if (!obj) + { + Repro(name + ": FAIL (GetClientObject()==null, sel=" + + wxString::Format("%d", sel) + ")"); + return; + } + + wxStringClientData *sd = static_cast(obj); + const wxString got = sd->GetData(); + const wxString want = ExpectedData(sel); + if (got == want) + Repro(name + ": PASS (" + got + ")"); + else + Repro(name + ": FAIL (got '" + got + "' want '" + want + "')"); +} + +class ReproFrame : public wxFrame +{ +public: + ReproFrame(); + +private: + void OnChoice(wxCommandEvent &e) { CheckEvent("choice_clientdata", e); } + void OnListBox(wxCommandEvent &e) { CheckEvent("listbox_clientdata", e); } + void OnComboBox(wxCommandEvent &e) { CheckEvent("combobox_clientdata", e); } +}; + +static void Fill(wxControlWithItems *ctrl) +{ + ctrl->Append("Alpha", new wxStringClientData("DATA_A")); + ctrl->Append("Beta", new wxStringClientData("DATA_B")); + ctrl->Append("Gamma", new wxStringClientData("DATA_C")); +} + +ReproFrame::ReproFrame() + : wxFrame(nullptr, wxID_ANY, "selection client-data repro", + wxDefaultPosition, wxSize(360, 320)) +{ + wxBoxSizer *sizer = new wxBoxSizer(wxVERTICAL); + + wxChoice *choice = new wxChoice(this, wxID_ANY); + Fill(choice); + choice->Bind(wxEVT_CHOICE, &ReproFrame::OnChoice, this); + sizer->Add(choice, 0, wxALL | wxEXPAND, 8); + + wxListBox *listbox = new wxListBox(this, wxID_ANY); + Fill(listbox); + listbox->Bind(wxEVT_LISTBOX, &ReproFrame::OnListBox, this); + sizer->Add(listbox, 1, wxALL | wxEXPAND, 8); + + // editable combobox -> in the DOM port + wxComboBox *combo = new wxComboBox(this, wxID_ANY, ""); + Fill(combo); + combo->Bind(wxEVT_COMBOBOX, &ReproFrame::OnComboBox, this); + sizer->Add(combo, 0, wxALL | wxEXPAND, 8); + + SetSizer(sizer); + +#ifdef __EMSCRIPTEN__ + CallAfter([] { EM_ASM({ console.log('[REPRO] selevent 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/apps/standalone/slider-scroll/slider-scroll_test.cpp b/tests/apps/standalone/slider-scroll/slider-scroll_test.cpp new file mode 100644 index 000000000..624d9a4e6 --- /dev/null +++ b/tests/apps/standalone/slider-scroll/slider-scroll_test.cpp @@ -0,0 +1,88 @@ +// wxSlider must fire the wxEVT_SCROLL_* family, not only wxEVT_SLIDER (DOM port). +// +// Bug (src/wasm/slider.cpp, OnDomEvent): +// On a DOM 'input' the slider builds ONLY a wxEVT_SLIDER command event: +// wxCommandEvent event(wxEVT_SLIDER, GetId()); +// ... +// // TODO(dom-phase-3): also fire the wxScrollEvent family +// // (wxEVT_SCROLL_THUMBTRACK/THUMBRELEASE/CHANGED). +// Native ports fire the wxEVT_SCROLL_* scroll-event family first, then +// wxEVT_SLIDER (src/gtk/slider.cpp). +// +// This breaks KiCad's colour picker: dialog_color_picker_base.cpp connects the +// brightness AND transparency sliders EXCLUSIVELY to wxEVT_SCROLL_* (-> +// OnChangeBrightness / OnChangeAlpha). In WASM, dragging them did nothing. +// +// This app binds a slider to wxEVT_SCROLL_THUMBTRACK and wxEVT_SCROLL_CHANGED +// (NOT wxEVT_SLIDER), exactly like the colour picker. The spec drives the real +// : +// RED (bug present): the scroll handlers never run -> no [REPRO] scroll lines. +// GREEN (fixed): thumbtrack + changed fire with the dragged value. + +#include "wx/wxprec.h" +#ifndef WX_PRECOMP + #include "wx/wx.h" +#endif + +#include "wx/slider.h" + +#ifdef __EMSCRIPTEN__ +#include +#endif + +static void Repro(const wxString& line) +{ +#ifdef __EMSCRIPTEN__ + EM_ASM({ console.log('[REPRO] ' + UTF8ToString($0)); }, + (const char *)line.utf8_str()); +#endif +} + +class ReproFrame : public wxFrame +{ +public: + ReproFrame(); + +private: + void OnThumbtrack(wxScrollEvent &e) + { Repro(wxString::Format("slider_thumbtrack: %d", e.GetPosition())); } + void OnChanged(wxScrollEvent &e) + { Repro(wxString::Format("slider_changed: %d", e.GetPosition())); } + // regression guard: the command event must keep working too + void OnCommand(wxCommandEvent &e) + { Repro(wxString::Format("slider_command: %d", e.GetInt())); } +}; + +ReproFrame::ReproFrame() + : wxFrame(nullptr, wxID_ANY, "wxSlider scroll-event repro", + wxDefaultPosition, wxSize(360, 140)) +{ + wxBoxSizer *sizer = new wxBoxSizer(wxVERTICAL); + + wxSlider *slider = new wxSlider(this, wxID_ANY, 0, 0, 100); + // The colour picker binds ONLY the scroll family — no wxEVT_SLIDER. + slider->Bind(wxEVT_SCROLL_THUMBTRACK, &ReproFrame::OnThumbtrack, this); + slider->Bind(wxEVT_SCROLL_CHANGED, &ReproFrame::OnChanged, this); + slider->Bind(wxEVT_SLIDER, &ReproFrame::OnCommand, this); + sizer->Add(slider, 0, wxALL | wxEXPAND, 16); + + SetSizer(sizer); + +#ifdef __EMSCRIPTEN__ + CallAfter([] { EM_ASM({ console.log('[REPRO] slider 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/apps/standalone/stattext-mnemonic/stattext-mnemonic_test.cpp b/tests/apps/standalone/stattext-mnemonic/stattext-mnemonic_test.cpp new file mode 100644 index 000000000..affc53146 --- /dev/null +++ b/tests/apps/standalone/stattext-mnemonic/stattext-mnemonic_test.cpp @@ -0,0 +1,57 @@ +// wxStaticText must consume the mnemonic '&' like the native ports (DOM port). +// +// Bug (src/wasm/stattext.cpp, WXSetVisibleLabel): +// The visible label is pushed to the DOM verbatim (wxDomSetText), so a wx +// mnemonic marker '&' shows up literally. Native ports strip/interpret it: +// wxGTK runs GTKConvertMnemonics() + gtk_label_set_text_with_mnemonic(), so +// "&File" displays as "File" and "&&" collapses to a single "&". +// +// KiCad has ~17 dialog labels that carry a '&' (pin properties, find/replace, +// defaults dialogs, ...) which render with a stray '&' in WASM. +// +// The app creates one wxStaticText whose label exercises both rules; the spec +// reads the rendered : +// RED (bug present): the shows "&Layer && Net". +// GREEN (fixed): the shows "Layer & Net". + +#include "wx/wxprec.h" +#ifndef WX_PRECOMP + #include "wx/wx.h" +#endif + +#ifdef __EMSCRIPTEN__ +#include +#endif + +class ReproFrame : public wxFrame +{ +public: + ReproFrame() + : wxFrame(nullptr, wxID_ANY, "wxStaticText mnemonic repro", + wxDefaultPosition, wxSize(320, 120)) + { + wxBoxSizer *sizer = new wxBoxSizer(wxVERTICAL); + // "&Layer" -> mnemonic 'L' removed -> "Layer"; "&&" -> "&". + sizer->Add(new wxStaticText(this, wxID_ANY, "&Layer && Net"), + 0, wxALL, 16); + SetSizer(sizer); + +#ifdef __EMSCRIPTEN__ + CallAfter([] { EM_ASM({ console.log('[REPRO] stattext-mnemonic 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/apps/standalone/textctrl-clear/textctrl-clear_test.cpp b/tests/apps/standalone/textctrl-clear/textctrl-clear_test.cpp new file mode 100644 index 000000000..824d48fc5 --- /dev/null +++ b/tests/apps/standalone/textctrl-clear/textctrl-clear_test.cpp @@ -0,0 +1,81 @@ +// wxTextCtrl::Clear()/Remove() must update the visible (DOM port). +// +// Bug (src/wasm/textentry.cpp + textctrl.cpp): +// wxTextCtrl overrides WriteText() and DoSetValue() to push the new value into +// the DOM element, but it does NOT override Remove(). wxTextEntryBase::Clear() +// is { Remove(0, GetLastPosition()); }, so both Clear() and Remove() fall +// through to wxTextEntry::Remove(), which only mutates the C++ m_value cache: +// +// m_value.erase(from, to - from); +// // TODO(dom-phase-2): mirror the new value into the DOM element. +// +// So after textCtrl->Clear() the cache (GetValue()) is empty but the +// still shows the old text — and the next keystroke reads the stale DOM value +// back, silently resurrecting the "deleted" text. Native ports delete the text +// in the live widget (gtk_editable_delete_text). +// +// The spec sets a known value, clicks Clear (then Remove), and checks the real +// element value: +// RED (bug present): the keeps the old text after Clear()/Remove(). +// GREEN (fixed): the reflects the cache. + +#include "wx/wxprec.h" +#ifndef WX_PRECOMP + #include "wx/wx.h" +#endif + +#ifdef __EMSCRIPTEN__ +#include +#endif + +enum { ID_CLEAR = wxID_HIGHEST + 1, ID_REMOVE }; + +class ReproFrame : public wxFrame +{ +public: + ReproFrame(); + +private: + void OnClear(wxCommandEvent &) { m_text->Clear(); } + // Remove the middle: "hello world" -> "ho world" (drops chars [1,5)). + void OnRemove(wxCommandEvent &) { m_text->Remove(1, 5); } + + wxTextCtrl *m_text = nullptr; +}; + +ReproFrame::ReproFrame() + : wxFrame(nullptr, wxID_ANY, "wxTextCtrl Clear/Remove repro") +{ + wxBoxSizer *sizer = new wxBoxSizer(wxVERTICAL); + + m_text = new wxTextCtrl(this, wxID_ANY, "hello world"); + sizer->Add(m_text, 0, wxALL | wxEXPAND, 10); + + wxButton *clearBtn = new wxButton(this, ID_CLEAR, "Clear"); + clearBtn->Bind(wxEVT_BUTTON, &ReproFrame::OnClear, this); + sizer->Add(clearBtn, 0, wxALL, 10); + + wxButton *removeBtn = new wxButton(this, ID_REMOVE, "Remove"); + removeBtn->Bind(wxEVT_BUTTON, &ReproFrame::OnRemove, this); + sizer->Add(removeBtn, 0, wxALL, 10); + + SetSizer(sizer); + +#ifdef __EMSCRIPTEN__ + CallAfter([] { EM_ASM({ console.log('[REPRO] textctrl-clear 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/apps/standalone/textsel/textsel_test.cpp b/tests/apps/standalone/textsel/textsel_test.cpp new file mode 100644 index 000000000..37d6eb85d --- /dev/null +++ b/tests/apps/standalone/textsel/textsel_test.cpp @@ -0,0 +1,96 @@ +// Reproduction for parity-audit H-3/#30: wxTextEntry caret/selection is a pure +// C++-side cache in the WASM DOM port. +// +// - READ (H-3): GetInsertionPoint()/GetSelection() never consult the DOM +// 's selectionStart/selectionEnd. Worse, every DOM "input" event +// routes through wxTextEntry::DoSetValue, which resets the cached caret to +// 0 and clears the cached selection — so after any typing (or a user +// drag-select) the accessors report insertion=0, sel=(0,0) regardless of +// the real caret. +// - WRITE (#30): SetInsertionPoint()/SetSelection()/SelectAll() update only +// the cache; nothing calls setSelectionRange() on the DOM element, so +// programmatic selection (e.g. KiCad's select-all-on-dialog-open) is +// invisible and typing inserts instead of replacing. +// +// The app exposes one wxTextCtrl seeded "hello world" plus three buttons: +// Report -> logs "[REPRO] textsel state: insertion= sel=," +// from the C++ accessors (read-path probe). +// Select Middle -> SetSelection(2, 7) (write-path probe) +// Select All -> SelectAll() (write-path probe) +// The spec (tests/e2e/parity-audit.spec.ts) manipulates/reads the real DOM +// selection and cross-checks both directions. + +#include "wx/wxprec.h" +#ifndef WX_PRECOMP + #include "wx/wx.h" +#endif + +#ifdef __EMSCRIPTEN__ +#include +#endif + +enum { ID_REPORT = wxID_HIGHEST + 1, ID_SEL_MIDDLE, ID_SEL_ALL }; + +class ReproFrame : public wxFrame +{ +public: + ReproFrame(); + +private: + void OnReport(wxCommandEvent &) + { + long from = 0, to = 0; + m_text->GetSelection(&from, &to); + const long insertion = m_text->GetInsertionPoint(); +#ifdef __EMSCRIPTEN__ + EM_ASM({ + console.log('[REPRO] textsel state: insertion=' + $0 + ' sel=' + $1 + ',' + $2); + }, (int)insertion, (int)from, (int)to); +#endif + } + void OnSelMiddle(wxCommandEvent &) { m_text->SetSelection(2, 7); } + void OnSelAll(wxCommandEvent &) { m_text->SelectAll(); } + + wxTextCtrl *m_text = nullptr; +}; + +ReproFrame::ReproFrame() + : wxFrame(nullptr, wxID_ANY, "wxTextEntry caret/selection repro") +{ + wxBoxSizer *sizer = new wxBoxSizer(wxVERTICAL); + + m_text = new wxTextCtrl(this, wxID_ANY, "hello world"); + sizer->Add(m_text, 0, wxALL | wxEXPAND, 10); + + wxButton *reportBtn = new wxButton(this, ID_REPORT, "Report"); + reportBtn->Bind(wxEVT_BUTTON, &ReproFrame::OnReport, this); + sizer->Add(reportBtn, 0, wxALL, 10); + + wxButton *middleBtn = new wxButton(this, ID_SEL_MIDDLE, "Select Middle"); + middleBtn->Bind(wxEVT_BUTTON, &ReproFrame::OnSelMiddle, this); + sizer->Add(middleBtn, 0, wxALL, 10); + + wxButton *allBtn = new wxButton(this, ID_SEL_ALL, "Select All"); + allBtn->Bind(wxEVT_BUTTON, &ReproFrame::OnSelAll, this); + sizer->Add(allBtn, 0, wxALL, 10); + + SetSizer(sizer); + +#ifdef __EMSCRIPTEN__ + CallAfter([] { EM_ASM({ console.log('[REPRO] textsel 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/parity-audit.spec.ts b/tests/e2e/parity-audit.spec.ts new file mode 100644 index 000000000..d9827de27 --- /dev/null +++ b/tests/e2e/parity-audit.spec.ts @@ -0,0 +1,269 @@ +import { test, expect, tryLoadApp } from './utils/fixtures'; + +// Red-green reproductions for wxWidgets DOM-port vs native (wxGTK) parity gaps +// found in the parity audit. Each standalone app +// (tests/apps/standalone//) drives the exact buggy path and self-reports a +// "[REPRO] : PASS/FAIL" line (or exposes live DOM state); the test is RED +// while the bug is present and GREEN after the wasm-layer fix. + +// Find the "[REPRO] : ..." line emitted by a repro app. +function reproLine(logs: string[], name: string): string | undefined { + return logs.find((l) => l.includes(`[REPRO] ${name}:`)); +} + +async function waitReady(testLogger: { consoleLogs: string[] }, marker: string) { + await expect + .poll(() => testLogger.consoleLogs.some((l) => l.includes(marker)), { + timeout: 30000, + message: `repro app should emit "${marker}"`, + }) + .toBe(true); +} + +test.describe('wxWidgets DOM-port parity reproductions', () => { + // C-1 (Critical): wxEVT_CHOICE/LISTBOX/COMBOBOX are hand-rolled and never call + // InitCommandEventWithItems(), so event.GetClientObject() is always NULL even + // when the item was appended WITH client data. KiCad's Track & Via Properties + // dialog static_cast(aEvent.GetClientData())->... then traps + // the WASM module on a normal selection. The app appends typed client data and + // the handler checks the event carries it. + test('selection events carry per-item client data (choice/listbox/combobox)', async ({ + page, + testLogger, + }) => { + await page.goto('/standalone/selevent-clientdata/selevent-clientdata_test.html'); + expect(await tryLoadApp(page, 30000), 'repro app should load').toBe(true); + await waitReady(testLogger, '[REPRO] selevent ready'); + + // wxChoice -> : pick index 1. + await page.locator('select[multiple]').first().selectOption({ index: 1 }); + await expect + .poll(() => reproLine(testLogger.consoleLogs, 'listbox_clientdata') ?? '', { + timeout: 10000, + message: 'wxListBox selection event must carry the item client object', + }) + .toContain('PASS'); + + // wxComboBox -> : commit "Beta" (index 1). + const combo = page.locator('input[list]').first(); + await combo.fill('Beta'); + await combo.dispatchEvent('change'); + await expect + .poll(() => reproLine(testLogger.consoleLogs, 'combobox_clientdata') ?? '', { + timeout: 10000, + message: 'wxComboBox selection event must carry the item client object', + }) + .toContain('PASS'); + }); + + // H-4 (High): wxTextCtrl does not override Remove(), so Clear() (=Remove(0,-1)) + // and Remove() update only the C++ cache and leave the showing the old + // text. The app sets "hello world"; buttons drive Remove(1,5) then Clear(). + test('wxTextCtrl Clear()/Remove() update the visible ', async ({ page, testLogger }) => { + await page.goto('/standalone/textctrl-clear/textctrl-clear_test.html'); + expect(await tryLoadApp(page, 30000), 'repro app should load').toBe(true); + await waitReady(testLogger, '[REPRO] textctrl-clear ready'); + + const input = page.locator('input').first(); + await expect + .poll(async () => await input.inputValue(), { timeout: 10000 }) + .toBe('hello world'); + + // Remove(1,5): "hello world" -> "h world". + await page.getByRole('button', { name: 'Remove' }).click(); + await expect + .poll(async () => await input.inputValue(), { + timeout: 10000, + message: 'Remove() must update the ', + }) + .toBe('h world'); + + // Clear(): -> "". + await page.getByRole('button', { name: 'Clear' }).click(); + await expect + .poll(async () => await input.inputValue(), { + timeout: 10000, + message: 'Clear() must empty the ', + }) + .toBe(''); + }); + + // H-5 (High): wxCheckListBox check marks are wiped on every item-list rebuild + // (Append re-pushes only m_itemsSelected, never m_itemsChecked). The app + // appends 5 rows and checks the even ones (0,2,4) with the Append-then-Check + // pattern; before the fix only the last-checked row survives in the DOM. + test('wxCheckListBox check marks survive item-list rebuilds', async ({ page, testLogger }) => { + await page.goto('/standalone/checklist-checks/checklist-checks_test.html'); + expect(await tryLoadApp(page, 30000), 'repro app should load').toBe(true); + await waitReady(testLogger, '[REPRO] checklist ready'); + + const boxes = page.locator('[data-wx-check-list] input[type=checkbox]'); + await expect.poll(async () => await boxes.count(), { timeout: 10000 }).toBe(5); + + const expected = [true, false, true, false, true]; // checked evens + await expect + .poll( + async () => { + const states: boolean[] = []; + for (let i = 0; i < 5; i++) states.push(await boxes.nth(i).isChecked()); + return JSON.stringify(states); + }, + { + timeout: 10000, + message: 'rows 0,2,4 must be checked; 1,3 unchecked', + }, + ) + .toBe(JSON.stringify(expected)); + }); + + // H-6 (High): wxSlider fires only wxEVT_SLIDER, never the wxEVT_SCROLL_* family + // — so KiCad's colour-picker brightness/alpha sliders (bound exclusively to + // wxEVT_SCROLL_*) do nothing. The app binds only the scroll family; the spec + // drives the real . + test('wxSlider fires the wxEVT_SCROLL_* family on drag', async ({ page, testLogger }) => { + await page.goto('/standalone/slider-scroll/slider-scroll_test.html'); + expect(await tryLoadApp(page, 30000), 'repro app should load').toBe(true); + await waitReady(testLogger, '[REPRO] slider ready'); + + const slider = page.locator('input[type=range]').first(); + await slider.evaluate((el: HTMLInputElement) => { + el.value = '70'; + el.dispatchEvent(new Event('input', { bubbles: true })); + el.dispatchEvent(new Event('change', { bubbles: true })); + }); + + // The scroll family must fire with the dragged value (RED before the fix). + await expect + .poll(() => reproLine(testLogger.consoleLogs, 'slider_thumbtrack') ?? '', { + timeout: 10000, + message: 'wxEVT_SCROLL_THUMBTRACK must fire with the slider value', + }) + .toContain('slider_thumbtrack: 70'); + await expect + .poll(() => reproLine(testLogger.consoleLogs, 'slider_changed') ?? '', { + timeout: 10000, + message: 'wxEVT_SCROLL_CHANGED must fire', + }) + .toContain('slider_changed: 70'); + }); + + // H-9 (High): wxConfig read-back truncates non-ASCII at the first multi-byte + // char (getConfigEntryLength/getConfigKeyLength return UTF-16 code-unit count, + // used as a UTF-8 byte budget). The app writes a non-ASCII value and entry + // name and reads them back in-session. + test('wxConfig round-trips non-ASCII strings without truncation', async ({ page, testLogger }) => { + await page.goto('/standalone/config-utf8/config-utf8_test.html'); + expect(await tryLoadApp(page, 30000), 'repro app should load').toBe(true); + await waitReady(testLogger, '[REPRO] config ready'); + + await expect + .poll(() => reproLine(testLogger.consoleLogs, 'config_value') ?? '', { + timeout: 10000, + message: 'non-ASCII config value must round-trip intact', + }) + .toContain('PASS'); + await expect + .poll(() => reproLine(testLogger.consoleLogs, 'config_keyname') ?? '', { + timeout: 10000, + message: 'non-ASCII config entry name must round-trip intact', + }) + .toContain('PASS'); + }); + + // #36 (Medium): wxStaticText pushes its label to the DOM verbatim, so a wx + // mnemonic '&' renders literally. Native ports consume it ("&File" -> "File", + // "&&" -> "&"). The app renders "&Layer && Net". + test('wxStaticText consumes the mnemonic ampersand', async ({ page, testLogger }) => { + await page.goto('/standalone/stattext-mnemonic/stattext-mnemonic_test.html'); + expect(await tryLoadApp(page, 30000), 'repro app should load').toBe(true); + await waitReady(testLogger, '[REPRO] stattext-mnemonic ready'); + + // wxStaticText -> ; it is the only span in this app. + await expect + .poll(async () => (await page.locator('span').first().textContent()) ?? '', { + timeout: 10000, + message: 'the rendered label must have the mnemonic & removed and && collapsed', + }) + .toBe('Layer & Net'); + }); + + // H-3/#30 (High): wxTextEntry caret/selection is a pure C++ cache. READ: the + // accessors never consult the DOM input's selectionStart/End, and every DOM + // "input" event resets the cached caret to 0 — so GetInsertionPoint()/ + // GetSelection() report 0/(0,0) regardless of the real caret. WRITE: + // SetSelection()/SelectAll() never call setSelectionRange(), so programmatic + // selection (KiCad's select-all-on-dialog-open) is invisible in the DOM. + // The app reports the C++ view on demand and offers SetSelection buttons. + test('wxTextEntry caret/selection is live against the DOM input', async ({ page, testLogger }) => { + await page.goto('/standalone/textsel/textsel_test.html'); + expect(await tryLoadApp(page, 30000), 'repro app should load').toBe(true); + await waitReady(testLogger, '[REPRO] textsel ready'); + + const input = page.locator('input').first(); + await expect.poll(async () => input.inputValue(), { timeout: 10000 }).toBe('hello world'); + + const stateLines = () => + testLogger.consoleLogs.filter((l) => l.includes('[REPRO] textsel state:')); + + // READ half (H-3), user selection: put a real selection on the DOM input, + // then ask C++ what it sees. + await input.evaluate((el: HTMLInputElement) => el.setSelectionRange(4, 9)); + await page.getByRole('button', { name: 'Report', exact: true }).click(); + await expect + .poll(() => stateLines()[0] ?? '', { + timeout: 10000, + message: 'GetInsertionPoint/GetSelection must reflect the live DOM selection (4,9)', + }) + .toContain('insertion=4 sel=4,9'); + + // READ half (H-3), typed caret: type at the end — the DOM caret is at 12, + // while the buggy cache is reset to 0 by the input event. + await input.click(); + await input.evaluate((el: HTMLInputElement) => el.setSelectionRange(11, 11)); + await input.pressSequentially('!'); + await expect.poll(async () => input.inputValue(), { timeout: 10000 }).toBe('hello world!'); + await page.getByRole('button', { name: 'Report', exact: true }).click(); + await expect + .poll(() => stateLines()[1] ?? '', { + timeout: 10000, + message: 'GetInsertionPoint must track the DOM caret after typing (12)', + }) + .toContain('insertion=12 sel=12,12'); + + // WRITE half (#30): programmatic SetSelection/SelectAll must reach the DOM. + // Mirror KiCad's real order (dialog_shim select-all runs while the field is + // unfocused, then the dialog focuses it): set selection via the button + // (which blurs the input to itself), then focus the input and read — the + // selection must survive the focus, which is what makes type-to-replace work. + const focusAndReadSelection = () => + input.evaluate((el: HTMLInputElement) => { + el.focus(); + return [el.selectionStart, el.selectionEnd]; + }); + + await page.getByRole('button', { name: 'Select Middle', exact: true }).click(); + await expect + .poll(focusAndReadSelection, { + timeout: 10000, + message: 'SetSelection(2,7) must set a DOM selection that survives focus', + }) + .toEqual([2, 7]); + + await page.getByRole('button', { name: 'Select All', exact: true }).click(); + await expect + .poll(focusAndReadSelection, { + timeout: 10000, + message: 'SelectAll() must select the whole DOM value', + }) + .toEqual([0, 12]); + }); +}); From 08218a265faf034fe7d0b7a55558b0e0e69ccbde Mon Sep 17 00:00:00 2001 From: Istvan Matejcsok <119620946+matejcsok-ee@users.noreply.github.com> Date: Tue, 7 Jul 2026 10:03:17 +0200 Subject: [PATCH 09/14] docs(wx-parity): native-parity audit, red/green log, e2e reachability The wxWidgets DOM-port vs native (wxGTK) parity audit plus the per-finding RED/GREEN measurements and KiCad-e2e reachability notes backing the C-1 / H-1..H-9 / #36 fixes. Co-Authored-By: Claude Opus 4.8 --- docs/features/wx-parity-fixes/README.md | 82 +++ .../wx-parity-fixes/kicad-e2e-reachability.md | 126 ++++ .../wx-parity-fixes/native-parity-audit.md | 619 ++++++++++++++++++ docs/features/wx-parity-fixes/redgreen.md | 375 +++++++++++ 4 files changed, 1202 insertions(+) create mode 100644 docs/features/wx-parity-fixes/README.md create mode 100644 docs/features/wx-parity-fixes/kicad-e2e-reachability.md create mode 100644 docs/features/wx-parity-fixes/native-parity-audit.md create mode 100644 docs/features/wx-parity-fixes/redgreen.md diff --git a/docs/features/wx-parity-fixes/README.md b/docs/features/wx-parity-fixes/README.md new file mode 100644 index 000000000..a579ac0e4 --- /dev/null +++ b/docs/features/wx-parity-fixes/README.md @@ -0,0 +1,82 @@ +# wxWidgets DOM-port ↔ native parity fixes (`wxwidgets-diff`) + +Six defects where the wxWidgets **WASM DOM port** diverges from native +wxWidgets (wxGTK), each found in a parity audit, reproduced with a failing +test, and fixed in the **wasm layer only** — no wxWidgets core file and no +KiCad source was changed. + +The common root cause is the DOM port's two-sided design (see +[`../wx-dom-port/README.md`](../wx-dom-port/README.md)): every control is a real +HTML element mirrored from a C++ state cache. Most of these bugs are the two +sides drifting apart, or the port skipping something native wxWidgets does. + +Status: **all six fixed and green** (2026-06-30), plus a **seventh fix, H-1**, +implemented and KiCad-verified (2026-07-03). **H-5**, **C-1**, and **H-1** are +additionally verified through real KiCad pcbnew dialogs (the Plot dialog, the +Track & Via Properties dialog, and the Page Settings dialog). Work lives on branch +`wxwidgets-diff` (root + wxwidgets submodule); not merged to `main`. + +## The fixes + +| # | Sev | Bug (native behavior → DOM-port bug) | Fix location | +|---|-----|---|---| +| **C-1** | Critical | `wxEVT_CHOICE`/`LISTBOX`/`COMBOBOX` selection events never attach per-item client data → `event.GetClientData()` is always NULL → KiCad's Track & Via Properties dialog dereferences NULL on a normal selection | `src/wasm/{choice,listbox,combobox}.cpp` — call `InitCommandEventWithItems(event, n)` | +| **H-4** | High | `wxTextCtrl::Clear()`/`Remove()` update only the C++ cache; the `` keeps the old text (next keystroke resurrects it) | `src/wasm/textctrl.{h,cpp}` — override `Remove()` to push to the DOM (covers `Clear()`) | +| **H-5** | High | `wxCheckListBox` check marks wiped on every item rebuild — an `Append`-then-`Check` loop loses every check but the last | `include/wx/wasm/listbox.h`, `src/wasm/checklst.{h,cpp}` — virtual `WasmSyncSelection`, override to re-apply `m_itemsChecked` | +| **H-6** | High | `wxSlider` fires only `wxEVT_SLIDER`, never the `wxEVT_SCROLL_*` family → handlers bound only to the scroll family (KiCad colour picker) never run | `src/wasm/slider.cpp` — fire `THUMBTRACK`/`CHANGED`/`THUMBRELEASE` | +| **H-9** | High | `wxConfig` read-back truncates non-ASCII at the first multi-byte char (UTF-16 `.length` used as a UTF-8 byte budget) | `build/wasm/wx.js` — length helpers return UTF-8 byte length | +| **#36** | Medium | `wxStaticText` renders the mnemonic `&` literally (`&File` shows an `&`) | `src/wasm/stattext.cpp` — `RemoveMnemonics()` before the label push | + +Plus a **seventh fix added later** (2026-07-03), a distinct audit finding that had +never been implemented — KiCad e2e only, no standalone repro: + +| # | Sev | Bug (native behavior → DOM-port bug) | Fix location | +|---|-----|---|---| +| **H-1** | High | `wxDialog::ShowModal` never created a `wxWindowDisabler`, so a "modal" dialog left the parent editor frame fully input-live (menubar/toolbar/canvas keep firing) | `src/wasm/dialog.cpp` — `new wxWindowDisabler(this)` in `ShowModal` (the upstream-univ mechanism the port had dropped) | + +Plus a **test-harness fix**: `tests/apps/Makefile.wasm` exports `stringToNewUTF8` +(the DOM string-read bridge needs it; the production build gets it via embind +`--bind`, the standalone apps don't). + +## Two test layers + +1. **Standalone wxWidgets e2e** — one tiny WASM app per bug under + `tests/apps/standalone/`, driven by `tests/e2e/parity-audit.spec.ts`. Each app + isolates the exact buggy path and self-reports `[REPRO] : PASS/FAIL` (or + exposes live DOM state). All six are red before the fix, green after. +2. **KiCad e2e (product proof)** — three specs drive real pcbnew dialogs: + `tests/kicad/plot-checklist.spec.ts` (the Plot dialog's layer checklist) for + **H-5**, `tests/kicad/via-clientdata.spec.ts` (the Track & Via Properties + dialog's predefined-via-size choice) for **C-1**, and + `tests/kicad/modal-input-lock.spec.ts` (the Page Settings dialog disabling the + main frame) for **H-1**. See + [`kicad-e2e-reachability.md`](kicad-e2e-reachability.md) for why the other four + of the original six resist a clean KiCad e2e — and why H-1's test must use a + *real*-modal dialog (Page Settings), not a quasi-modal one (Plot / Track & Via + already disable the parent via KiCad's own `WINDOW_DISABLER`). + +See [`redgreen.md`](redgreen.md) for the exact measured red→green ledger. + +## Build & run + +```bash +# 1. Build the patched wxWidgets (standalone) and the repro apps +./scripts/build-wx-wasm.sh +cd tests/apps && make -f Makefile.wasm parity-repros stattext-ellipsize && cd ../.. + +# 2. Standalone parity tests +cd tests && npx playwright test e2e/parity-audit.spec.ts + +# 3. KiCad e2e for H-5 (needs a pcbnew build) +BINARYEN_OPT_LEVEL=-O1 ./docker/build.sh pcbnew # -O1: never -O2 +cd tests && npm run setup:kicad +npx playwright test --config=playwright-kicad.config.ts --project=chromium --headed kicad/plot-checklist.spec.ts +``` + +## Files + +- Fixes: `wxwidgets/{src/wasm/*.cpp, include/wx/wasm/*.h, build/wasm/wx.js}` (11 files). +- Standalone repros: `tests/apps/standalone/{selevent-clientdata,textctrl-clear,checklist-checks,slider-scroll,config-utf8,stattext-mnemonic}/`. +- H-1 fix: `wxwidgets/src/wasm/dialog.cpp` (`wxWindowDisabler` in `ShowModal`). +- Specs: `tests/e2e/parity-audit.spec.ts`; KiCad e2e `tests/kicad/{plot-checklist,via-clientdata,modal-input-lock}.spec.ts`. +- Patches snapshot: `features/wxwidgets-diff/{root,wxwidgets}.patch`. diff --git a/docs/features/wx-parity-fixes/kicad-e2e-reachability.md b/docs/features/wx-parity-fixes/kicad-e2e-reachability.md new file mode 100644 index 000000000..81d766508 --- /dev/null +++ b/docs/features/wx-parity-fixes/kicad-e2e-reachability.md @@ -0,0 +1,126 @@ +# KiCad e2e reachability of the parity bugs + +All six fixes are proven red→green by the **standalone** wxWidgets e2e tests. A +separate question is which can *also* be reproduced through a **real KiCad app** +as a product-level e2e test. This documents that investigation (done against a +live pcbnew build) so nobody has to re-derive it. + +**Answer:** **H-5**, **C-1**, and **H-1** have product-level KiCad e2e proofs +(`tests/kicad/plot-checklist.spec.ts`, `tests/kicad/via-clientdata.spec.ts`, +`tests/kicad/modal-input-lock.spec.ts`). The remaining four of the original six +each have a specific, verified blocker. + +> **Update (2026-07-01):** C-1 was originally logged here as "Blocked-ish". That +> was overcautious — it *is* reachable and cleanly DOM-assertable. See the C-1 row +> and "How C-1 is reproduced" below. +> +> **Update (2026-07-03):** **H-1** (a separate audit finding, not one of the six — +> "modal dialogs are not input-modal") was **implemented** and given a KiCad e2e. +> The audit called it unsurfaceable ("e2e tests drive only the dialog"); it isn't. +> See "How H-1 is reproduced" below — the catch is that most KiCad dialogs are +> *quasi-modal* and already disable the parent, so the test must pick a real +> `wxDialog::ShowModal` dialog (Page Settings). + +## Which pcbnew dialogs actually open in WASM + +Probed by driving the File menu and inspecting the DOM: + +| Dialog | Opens? | Notable widgets seen | +|---|---|---| +| **Plot** (`File → Plot…`) | ✅ | `wxCheckListBox` (48 layer rows), choices, text fields | +| **Print** (`File → Print…`) | ✅ | `wxCheckListBox`, `m_colorTheme` `wxChoice` with client data | +| **Board Setup** (`File → Board Setup…`) | ✅ | 13 choices, 28 text fields (treebook of panels) | +| **Page Settings** (`File → Page Settings…`) | ✅ | 16-opt page-size choice, ~21 title-block text fields | +| **Colour picker** (swatch → picker) | ❌ | does not open in WASM (blocks H-6 via that path) | + +Menus (menubar + popups) are always present. + +## Per-bug verdict + +| Bug | Reachable surface | Cleanly assertable? | Verdict | +|---|---|---|---| +| **H-5** checklist | Plot dialog ✅ | ✅ count checked `[data-wx-check-list] input:checked` | **DONE** — `tests/kicad/plot-checklist.spec.ts` | +| **C-1** client-data | Track & Via Properties dialog | ✅ Via-diameter field value | **DONE** — `tests/kicad/via-clientdata.spec.ts`. Print's `m_colorTheme` is read via the *control* (`GetClientData(n)`), not the event, so it does *not* exercise C-1; the via dialog's `onViaSelect` is the one true event-path consumer (an exhaustive tree sweep found exactly one). | +| **#30** caret/`SelectAll` | UNIT_BINDER numeric fields (everywhere) | ✅ `input.selectionStart/End` | **Fragile**: KiCad only calls `SelectAll()` on *indeterminate multi-select* or *validation error* (`unit_binder.cpp:233,302,395,410`) — not on plain focus. Needs a specific multi-select or out-of-range setup. This one is *unfixed* (deferred backlog) so it would be RED on the current build (1 build to fix). | +| **H-6** slider | Appearance panel's 6 `STEPPED_SLIDER` opacity sliders (always visible) | ❌ effect is a **canvas opacity change** | **Blocked**: only assertable by canvas pixel-diff (inherently flaky) — same problem as the colour picker. | +| **#36** stattext `&` | — | ✅ span text | **Unreachable**: KiCad `&` mnemonics are on buttons/menu items, **not `wxStaticText`**. No `&` static label in any dialog that opens (Plot/Print/Page Settings all checked). | +| **H-4** textctrl Clear | needs a dialog with Clear/Reset on a text field | ✅ input value | **No clean surface found** in the dialogs that open. | +| **H-9** config UTF-8 | — | — | **Unreachable**: no UI action writes then reads back a non-ASCII config value. | + +## Why H-5 works where Print's checklist doesn't + +The bug only manifests when `Append` and `Check` are **interleaved** in one loop: +each `Append` rebuilds the DOM rows unchecked, so a `Check` from an earlier +iteration is wiped by a later `Append`. + +- **Plot** (`dialog_plot.cpp:351-354`): `int i = Append(name); if(sel) Check(i);` + in one loop → interleaved → **bug manifests** (0 of 48 checked). +- **Print** (`dialog_print_pcbnew.cpp:118` then `:156`): appends *all* layers in + the constructor, then checks selected layers in a *separate* loop in + `TransferDataToWindow()` → no `Append` after the `Check`s → **bug does not + manifest**. + +So Print is not a valid second H-5 surface — a useful confirmation that the fix's +trigger is understood precisely. + +## How C-1 is reproduced (`tests/kicad/via-clientdata.spec.ts`) + +The perceived fragility came from two assumptions that turned out to be avoidable: + +- **"needs a configured predefined via size"** — true, but the list is seeded + straight from the board file: `(setup (user_via ) …)` populates + `m_ViasDimensionsList` (`pcb_io_kicad_sexpr_parser.cpp:2585`), so the dialog's + `m_predefinedViaSizesCtrl` choice is pre-filled on load — no Board Setup clicks. +- **"needs board + via + fragile 5-step canvas interaction"** — the board contains + *only* the via, so **Edit → Select All (Ctrl+A)** selects exactly the via (which + satisfies the tracks/vias-only condition `EDIT_TOOL::Properties` requires) with no + canvas-pixel math. Press **E** to open the dialog. + +The rest is a clean DOM assertion. The via's own diameter is `0.7 mm` (distinct from +the two predefined sizes and from 0). Picking predefined `"0.9 / 0.45"` fires the +`` goes `0.7 → 0.9`. +- **RED (bug):** `GetClientData()` is NULL, so `viaDimension->m_Diameter` reads 0 → + the field goes `0.7 → 0` (memory `wasm-null-deref-reads-zero`). + +The symptom is *silent wrong values*, not a crash — so the assertion is the field +value, plus a WASM-abort guard for the (rarer) trap case. + +## How H-1 is reproduced (`tests/kicad/modal-input-lock.spec.ts`) + +H-1: `wxDialog::ShowModal` never created a `wxWindowDisabler`, so a "modal" dialog +left the parent editor frame fully input-live. The audit deemed this unsurfaceable +by e2e ("e2e tests drive only the dialog"). Two facts make it surfaceable — and one +made the first attempt a false green: + +- **Signal.** Disabling the parent frame flips its `IsEnabled()`, and the port + re-emits `enabled` into `window.wxElementRegistry` on every `DoEnable()` + (`window.cpp:1438`). So the pcbnew main frame's registry entry flips + `enabled: true → false` the instant a real modal opens — and that same + `IsEnabled()` walk is exactly what every input gate consults. The test just reads + `registry.getElement().enabled` before / during / after the modal. +- **Dialog choice is load-bearing** (the trap). Most KiCad dialogs are opened via + KiCad's **`ShowQuasiModal()`**, which runs its own nested loop and *already* + creates a `WINDOW_DISABLER(parent)` (`dialog_shim.cpp:1431`) — so the parent is + disabled with or without the wx fix. **Plot** (`board_editor_control.cpp:565`) and + **Track & Via Properties** (`edit_tool.cpp:2306`) are both quasi-modal: driving + Plot on the *unpatched* binary already reported the frame disabled (a false green + that flushed this out). The test therefore drives **Page Settings**, shown via a + *real* `wxDialog::ShowModal()` (`board_editor_control.cpp:534`; `DIALOG_SHIM::ShowModal` + forwards straight through, adding no disabler of its own). So H-1's real user + impact in KiCad is limited to the real-`ShowModal` dialogs. + +Measured RED (frame `enabled` stays `true` while Page Settings is open → fail) / +GREEN (`false` while open, `true` after close → pass); see `redgreen.md`. Residual: +a menubar dropdown still *visually* opens over the modal (pure-JS popup, no C++ +gate) though its items do nothing — cosmetic, would need a DOM shield, out of scope. + +## Recommendation + +The cleanly-reachable Critical/High surfaces now have product-level proofs: +**H-5** (`plot-checklist.spec.ts`), **C-1** (`via-clientdata.spec.ts`), and **H-1** +(`modal-input-lock.spec.ts`, which also *implemented* the fix). The remaining four +of the original six are best left to the standalone suite (their KiCad surfaces need +a validation-error setup, a canvas pixel-diff, or don't exist — see the table). diff --git a/docs/features/wx-parity-fixes/native-parity-audit.md b/docs/features/wx-parity-fixes/native-parity-audit.md new file mode 100644 index 000000000..3336034f9 --- /dev/null +++ b/docs/features/wx-parity-fixes/native-parity-audit.md @@ -0,0 +1,619 @@ +# wxWidgets WASM/DOM Port — Parity Audit vs Native (wxGTK) + +*Definitive engineering report for the maintainer of an in-browser KiCad (PCBJam). 244 verified/carried findings folded with three inventory ground-truths (compiled-sources coverage map, setup.h honesty audit, DOM-bridge architecture). Every medium-and-above finding survived an adjudicator that read both the WASM and native/contract source; low items are carried (tagged).* + +--- + +## 1. Executive summary + +The wxWidgets WASM/DOM port is a real, broadly complete toolkit — not a thin shim. It is a genuine `TOOLKIT=WASM` build that compiles the full `GUI_CMN_SRC` set (grid, dataview, propgrid, AUI, STC, scrolled windows, all generic dialogs) into a 293-member core archive, plus 62 hand-written `src/wasm/*.cpp` port files, all verified present in the shipped `.a` files. The DOM-bridge design (three handle namespaces, fresh-entry event re-entry under an Asyncify-suspended stack, LIFO nested-modal pumps) is sound and is what makes in-browser KiCad work at all. **That said, real defects exist** — this is not a "zero findings" situation, and the prior report's "0 confirmed findings" headline was a tooling artifact, not the truth. + +The gaps cluster into a small number of root causes rather than 244 independent bugs. The single highest-risk item is a **critical null-dereference crash**: selection command events (`wxEVT_CHOICE`/`wxEVT_COMBOBOX`/`wxEVT_LISTBOX`) are hand-rolled and never attach per-item client data, so KiCad's Track & Via Properties dialog traps the WASM module on a normal user action. The largest *class* of real gaps is **state desynchronization between C++ caches and the live DOM** — text-control caret/selection/clipboard, menu/toolbar enable-check-label staleness (no `wxEVT_MENU_OPEN`, `wxMenuItem` is a pure data stub), and check-list/radio state — where the C++ side is right but the user sees stale or wrong UI. A second structural class is **the Canvas2D backend lacking raster-ops and clip-box state** (XOR/`wxINVERT` no-op, `SetClippingRegion` silently ignored, masked/stretched blits broken). Beyond correctness, **modal dialogs are not input-modal** (no `wxWindowDisabler` is ever created), which is a genuine re-entrancy/use-after-free hazard on the Asyncify-parked stack. Rounding out the picture: partial event families (no horizontal wheel, no slider scroll-event family, no `wxEVT_CHAR` to validators), HiDPI quantized to {1×, 2×}, no working printing, accessibility essentially absent, and a `setup.h` that misrepresents the shipped configuration. None of the medium/low items are crashes; most are degraded-UX or latent. The honest verdict: **functionally usable and impressively far along, with one must-fix crash, a coherent set of attackable state-sync root causes, and a long tail of cosmetic/inherent-platform items.** + +--- + +## 2. Severity scoreboard + +| Severity | Count | +|---|---| +| Critical | 1 | +| High | 11 (9 distinct after dedup: #11→#2, #12→#8) | +| Medium | 58 (≈50 distinct; inv-stub-todo #58–64,#69 duplicate higher items) | +| Low / carried | 174 | +| **Total** | **244** | + +Dismissed/refuted candidates (not counted above): **11** (see §10). + +### Subsystem × severity matrix + +| Subsystem | Crit | High | Med | Low | Total | +|---|---:|---:|---:|---:|---:| +| choices-lists | 1 | 1 | 1 | 4 | 7 | +| textctrl | – | 2 | 3 | 3 | 8 | +| menus | – | 2 | 3 | 1 | 6 | +| dc-core | – | 1 | 3 | 7 | 11 | +| toplevel-frame-dialog | – | 1 | 2 | 3 | 6 | +| static-range (slider/static) | – | 1 | 1 | 7 | 9 | +| timer-config-settings | – | 1 | 4 | 3 | 8 | +| critic-uncovered | – | 2 | 1 | 2 | 5 | +| window-core | – | – | 3 | 4 | 7 | +| input | – | – | 3 | 9 | 12 | +| fonts | – | – | 3 | 4 | 7 | +| cc-accessibility | – | – | 3 | 5 | 8 | +| bitmap-image | – | – | 2 | 7 | 9 | +| gdi-resources | – | – | 2 | 2 | 4 | +| glcanvas | – | – | 2 | 6 | 8 | +| cc-hidpi-scaling | – | – | 2 | 1 | 3 | +| containers | – | – | 1 | 10 | 11 | +| clipboard-dnd | – | – | 1 | 6 | 7 | +| buttons | – | – | 1 | 6 | 7 | +| cc-printing | – | – | 1 | 4 | 5 | +| cc-net-ipc-process | – | – | 1 | 4 | 5 | +| cc-locale-mime-stdpaths-fs | – | – | – | 7 | 7 | +| tooltip-misc | – | – | – | 3 | 3 | +| app-eventloop | – | – | – | 8 | 8 | +| inv-known-limitations | – | – | 2 | 22 | 24 | +| inv-stub-todo | – | – | 13 | 36 | 49 | +| **Total** | **1** | **11** | **58** | **174** | **244** | + +--- + +## 3. Critical & High findings + +### C-1 — Selection command events carry no per-item client data → null-deref crash in KiCad (#1) +- **Severity:** Critical +- **Gap:** `wxEVT_CHOICE`/`wxEVT_COMBOBOX`/`wxEVT_LISTBOX` are constructed by hand and never call `InitCommandEventWithItems`/`SetClientData`/`SetClientObject`. +- **Evidence:** `src/wasm/choice.cpp:221-239` (sets only `SetInt`/`SetString`/`SetEventObject`), `src/wasm/combobox.cpp:107-127`, `src/wasm/listbox.cpp:289-321`. Native contract: `src/common/ctrlsub.cpp:276-302` (`InitCommandEventWithItems`/`SendSelectionChangedEvent` copy `GetClientObject(n)`/`GetClientData(n)`), `src/common/lboxcmn.cpp:186-201`; ports route through these (`src/gtk/choice.cpp:31-34`, `src/gtk/combobox.cpp:31-35`). +- **Native vs WASM:** Native always attaches the selected item's per-item client data to the event; the WASM hand-rolled events never do, so `event.GetClientData()/GetClientObject()` always return NULL. +- **Impact:** Any handler reading the event's client data gets NULL. `pcbnew/dialogs/dialog_track_via_properties.cpp:963-968 onViaSelect` does `static_cast(aEvent.GetClientData())->m_Diameter` with no null guard → **null pointer dereference / WASM module trap** on a normal action (double-click a track/via, pick a predefined via size). The trap is unrecoverable. +- **KiCad relevance:** Confirmed crash path in a core, easily-reached pcbnew dialog (`dialog_track_via_properties_base.cpp:278/660` wires `wxEVT_COMMAND_CHOICE_SELECTED`; `:424/:525` populate via `Append(msg, viaDimension)`). All `Append(item, clientData)` + read-event-client-data handlers are equally affected. +- **Verifier note:** Confirmed on both sides; no base/generic/DOM/JS-shim path attaches the data because the WASM handlers bypass `SendSelectionChangedEvent`/`SendEvent`. Fix: replace the hand-rolled events with `SendSelectionChangedEvent(wxEVT_*)` (or call `InitCommandEventWithItems` before `HandleWindowEvent`). + +--- + +### H-1 — Modal dialogs are not input-modal; no `wxWindowDisabler` is ever created (#2, merges #11) +- **Severity:** High +- **Gap:** `ShowModal()` never disables other top-level windows; the port replaces a file (`src/univ/dialog.cpp`) that did this in one line. +- **Evidence:** `src/wasm/dialog.cpp:270-302` (`ShowModal` = set flag, `Show(true)`, `startModal()`); `m_windowDisabler` is NULL-inited at `dialog.cpp:59`, `wxDELETE`d at `:157`, and **never `new`'d** (grep clean). Native: `src/univ/dialog.cpp:193 m_windowDisabler = new wxWindowDisabler(this)`; `wxWindowDisabler::DoDisable` (`src/common/utilscmn.cpp:1555`) disables every other shown TLW; wxGTK uses `gtk_window_set_modal` (`src/gtk/dialog.cpp:159`). +- **Native vs WASM:** Native locks input to the dialog until `EndModal`; WASM leaves the parent frame's menubar/toolbar/canvas/controls fully enabled. The gate that *would* block them — `domevents.cpp:94 if(!window->IsEnabled()) return;` and the `IsEnabled()` checks in `app.cpp:152/187/298/367` — never trips because nothing is disabled. There is also no DOM-level backdrop/`showModal()`/`pointer-events` shield (`wx.js` only does z-index stacking). +- **Impact:** While a "modal" dialog is parked under Asyncify the user can still click the GAL canvas (firing tools), menubar titles, and toolbar buttons → re-entrant dialog/tool invocation, editing the document behind a settings dialog, or `Close()`/`Destroy()` of the parent while a child's `ShowModal` frame is suspended → **use-after-free / crash**. This is exactly the nested/consecutive-modal Asyncify-corruption class the dialog.cpp comments warn about. +- **KiCad relevance:** Every modal dialog (Board Setup, Preferences, DRC, Plot, Net Classes…). e2e tests drive only the dialog, so they don't surface it. +- **Verifier note:** Confirmed; the enforcement machinery is wired and waiting — the one-line fix (create the disabler) would immediately engage the existing `IsEnabled()` guards. Related low items: focus is also not saved/restored across a modal (#186). +- **STATUS (2026-07-03):** FIXED — `wxwidgets/src/wasm/dialog.cpp` now does `m_windowDisabler = new wxWindowDisabler(this)` in `ShowModal`. Reproduced by a KiCad e2e (`tests/kicad/modal-input-lock.spec.ts`), measured RED (parent frame `enabled` stays `true` during a modal) → GREEN (`false`). Nuance this entry missed: the fix only changes behavior for *real* `wxDialog::ShowModal` dialogs (e.g. **Page Settings**). Most KiCad dialogs are opened via **`ShowQuasiModal()`**, which already disables the parent through KiCad's own `WINDOW_DISABLER` (`dialog_shim.cpp:1431`) — so "Plot / Board Setup / DRC…" listed above are *not* affected in practice (they were already input-modal). See `redgreen.md`. + +--- + +### H-2 — `SetClippingRegion(x,y,w,h)` is a silent no-op (#3) +- **Severity:** High +- **Gap:** The rectangular DC clip reads clip members that the modern base class never populates, so the requested rectangle is never applied. +- **Evidence:** `src/wasm/dc.cpp:292-304` calls `wxDCImpl::DoSetClippingRegion(x,y,w,h)` then feeds `m_clipX1..m_clipY2` to JS `clipRect`. Base `src/common/dcbase.cpp:371-411` stores the box in `m_devClipX1..` (device units) and **never writes** `m_clipX1..` (ctor inits them to 0 at `:343`); `include/wx/dc.h:766` documents that the derived port must fill `m_clipX1..` itself (gtk/msw/qt/x11/dfb/dcpsg all do; wasm does not). JS `clipRect` (`wx.js:1194-1199`) treats `width<=0` as "uninitialized" and resets the clip to the full context. +- **Native vs WASM:** GTK applies the intersected rectangle as a real GC clip; WASM passes `clipRect(id, 0,0,0,0)` → clip reset to whole context. +- **Impact:** Every `wxDC`-level rectangular clip is ignored → silent overdraw/text bleed. (The `wxRegion` overload works; only the `(x,y,w,h)` coordinate overload is broken.) +- **KiCad relevance:** `wxGrid` cells, generic `wxDataViewCtrl`/`wxPropertyGrid` renderers, AUI tab/caption art, owner-drawn controls — all compiled-in and reachable; e.g. long cell text bleeds into neighbors. +- **Verifier note:** Confirmed; `m_clipX1` is only ever read in `src/wasm`, written nowhere. Fix: populate `m_clipX1..m_clipY2` in the port (as the other ports do). +- **STATUS (2026-07-03):** FIXED — `wxwidgets/src/wasm/dc.cpp` `DoSetClippingRegion` now mirrors the base's device-unit clip box back into `m_clipX1..m_clipY2` via `DoGetClippingRect()` (and `DestroyClippingRegion` chains to the base). Reproduced by a KiCad e2e (`tests/kicad/grid-clip-bleed.spec.ts`): Board Setup → Net Classes, 70-char netclass name next to an empty Clearance cell; measured RED (325 ink pixels bleeding across the neighbor) → GREEN (0, text clipped at the cell edge; name-cell ink identical 413/413). Nuance this entry missed: the *Text Variables* grid can't repro it — `WX_GRID::SetupColumnAutosizer` auto-fits the column to its content, masking the missing clip; Net Classes has fixed widths. See `redgreen.md`. + +--- + +### H-3 — Text-control caret/selection accessors return a stale cache reset to 0 on every keystroke (#4, merges #62) +- **Severity:** High +- **Gap:** `GetInsertionPoint`/`GetSelection`/`GetStringSelection`/`HasSelection` return a pure C++ cache that is hard-reset to 0 on each typed character and never reads the real DOM caret/selection. +- **Evidence:** `src/wasm/textentry.cpp:139-143` (`GetInsertionPoint`), `:180-199` (`GetSelection`); reset path `textctrl.cpp:109-120` (`OnDomEvent` INPUT) → `textentry.cpp:220-232` (`DoSetValue` sets `m_insertionPoint=0`, `m_selectionStart/End=-1`). There is **no DOM bridge** to read `selectionStart/selectionEnd` (grep of `dom.h`/`wx-dom.js` returns nothing). Native: `src/gtk/textentry.cpp:771-774` (`gtk_editable_get_position`), `:818-843` (`gtk_editable_get_selection_bounds`). +- **Native vs WASM:** GTK queries the live widget; WASM returns cache that is wrong after any typing or mouse/arrow caret move. +- **Impact:** After typing `abc` (DOM caret at 3) `GetInsertionPoint()` returns 0 and `GetSelection()` returns (0,0). Because `CanCopy/CanCut` are built on `HasSelection()` (`textentrycmn.cpp:303-311`), Edit-menu Copy/Cut enable state is also wrong; insert-at-cursor lands at 0 (see H-4/#31). +- **KiCad relevance:** Pervasive — live-filter/eval `EVT_TEXT` handlers, select-on-focus, clipboard-UI enable logic. Degraded, not crashing (native browser editing still works visually). +- **Verifier note:** Confirmed; the only DOM mirror is value (`wxDomGetValue/SetValue`). No selection read-back exists anywhere in the port. +- **STATUS (2026-07-06):** FIXED (covers #30 SetSelection/SelectAll-not-pushed and #31 WriteText-at-stale-caret). Added a DOM caret/selection bridge — `wxDomGetSelectionStart/End` + `wxDomSetSelection` in `include/wx/wasm/private/dom.h` + `build/wasm/wx-dom.js` — and made `src/wasm/textentry.cpp` live-read/write it (cache fallback pre-Create); `WriteText` now inserts at the live caret. **Required a one-line KiCad change too:** `SelectAllInTextCtrls`'s `SelectAll()` was gated `#if defined(__WXMAC__)||defined(__WXMSW__)` (`common/dialog_shim.cpp:901`) and thus compiled out for the wasm toolkit — widened with `|| defined(__WXWASM__)`. Reproduced by a KiCad e2e (`tests/kicad/dialog-select-on-open.spec.ts`): Track Properties opens with the width field focused + pre-filled `0.25`, typing `5` measured RED `"0.255"` (inserted) → GREEN `"5"` (replaced). Read-path proven by the standalone `textsel` app (no KiCad-observable consumer). Load-bearing nuance: a `setSelectionRange` on a *blurred* input is dropped on the next focus, so `wxDomSetSelection` registers a one-shot focus listener to re-apply it — matching KiCad's select-then-focus order. See `redgreen.md`. + +--- + +### H-4 — `Remove()`/`Clear()`/`RemoveSelection()` update the C++ value cache but never push to the DOM → visible text desync (#5, merges #60) +- **Severity:** High +- **Gap:** Base `wxTextEntry::Remove` erases `m_value` only; `wxTextCtrl` does not override `Remove`/`Clear`, so `wxDomSetValue` is never called. +- **Evidence:** `src/wasm/textentry.cpp:73-93` (`// TODO(dom-phase-2): mirror the new value into the DOM element.`); `include/wx/wasm/textctrl.h` overrides only `WriteText`/`OnDomEvent`/`DoSetValue`. Native: `src/gtk/textentry.cpp:695-698` (`gtk_editable_delete_text`); base `Clear(){Remove(0,-1);}` (`wx/textentry.h:60`). +- **Native vs WASM:** GTK visibly empties/shortens the control; WASM leaves the `/