From 6431b67dd506f0ebf90bc5a7663a7c90fb7529c9 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 1/3] =?UTF-8?q?test(kicad):=20C-1=20e2e=20=E2=80=94=20pred?= =?UTF-8?q?efined=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 b13c4fa5e..730b88627 160000 --- a/wxwidgets +++ b/wxwidgets @@ -1 +1 @@ -Subproject commit b13c4fa5ed61d46ee28bd3ef3a9b788debfd34f7 +Subproject commit 730b88627d32bd427f27fd64dad60d19368649fe From 020726e80977d4414a3e579a1c88f4a2ef429824 Mon Sep 17 00:00:00 2001 From: Istvan Matejcsok <119620946+matejcsok-ee@users.noreply.github.com> Date: Fri, 10 Jul 2026 11:33:04 +0200 Subject: [PATCH 2/3] test(standalone): C-1 selection client-data reproduction Standalone Emscripten test app driving the exact buggy path: wxChoice/ wxListBox/wxComboBox items appended WITH client data; the selection handler checks the command event carries it (InitCommandEventWithItems), self-reporting [REPRO] : PASS/FAIL to the console. Plus its Makefile.wasm target and the Playwright parity-audit spec that drives all three widgets. Extracted from the wxwidgets-diff parity-fix branch (e8f3b4c slice: C-1 only). Co-Authored-By: Claude Fable 5 --- tests/apps/Makefile.wasm | 18 ++- .../selevent-clientdata_test.cpp | 137 ++++++++++++++++++ tests/e2e/parity-audit.spec.ts | 67 +++++++++ 3 files changed, 220 insertions(+), 2 deletions(-) create mode 100644 tests/apps/standalone/selevent-clientdata/selevent-clientdata_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..814c9aec7 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,8 @@ 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 # Main test app minimal_test.o: minimal_test.cpp @@ -270,6 +271,19 @@ $(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 +.PHONY: selevent-clientdata + # Menu test (no GL) $(S)/menu/menu_test.o: $(S)/menu/menu_test.cpp $(CXX) -c $(CXXFLAGS) $< -o $@ 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/e2e/parity-audit.spec.ts b/tests/e2e/parity-audit.spec.ts new file mode 100644 index 000000000..5a6bccf77 --- /dev/null +++ b/tests/e2e/parity-audit.spec.ts @@ -0,0 +1,67 @@ +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'); + }); +}); From 62316e85a3d0389de694dc7fd0cdd74628e84c29 Mon Sep 17 00:00:00 2001 From: Istvan Matejcsok <119620946+matejcsok-ee@users.noreply.github.com> Date: Fri, 10 Jul 2026 17:01:06 +0200 Subject: [PATCH 3/3] =?UTF-8?q?fix(wasm):=20bump=20wxwidgets=20=E2=80=94?= =?UTF-8?q?=20survive=20throwing=20event=20handlers=20(dom-port-bugs)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-pick of the wxwidgets-diff EH fix (wx cfadc51cc2, orig ac1427d156): wxApp::OnExceptionInMainLoop() returns true so a throwing event handler no longer reaches WXConsumeException -> ExitMainLoop(), which under native wasm-EH destroys the top-level window (blank page). Needed here because the dom-port-bugs e2e on main exercises exactly this path and fails without it (PR #35 CI run 29090433643). Co-Authored-By: Claude Fable 5 --- wxwidgets | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wxwidgets b/wxwidgets index 730b88627..cfadc51cc 160000 --- a/wxwidgets +++ b/wxwidgets @@ -1 +1 @@ -Subproject commit 730b88627d32bd427f27fd64dad60d19368649fe +Subproject commit cfadc51cc265543a4106554e109ed2e434f29eac