wxWidgets DOM-port parity fixes (C-1…H-9, #36, H-8) + KiCad e2e

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 `<input>` 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] <name>: 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`.

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 <dia_mm> <drill_mm>) …)` 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
+`<select>` `change` → `wxChoice::OnDomEvent` → `onViaSelect`, which reads
+`event.GetClientData()`:
+
+- **GREEN (fixed):** the Via-diameter `<input>` 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(<PcbFrame id>).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).