Skip to content

Commit a6da71d

Browse files
u8arrayu8array
committed
refactor: harden printer-settings UI inputs against silent-rollback
Gemini PR review plus four review-iterations surfaced a class of bugs in the printer-settings tabs: typing or pasting a char the schema rejects (^/~/,/control) causes patchPrinterProfile to roll back, and the controlled input visibly freezes. The fix-attempt in the original feat commit only handled the maintenance message field and used a regex without the global flag, so only the first unsafe char was stripped. The same bug class already shipped in IdentityTab (printerName, printerDescription) and EncodingAndLanguageTab (encodingTable). This refactor: - Introduces `SafeStringInput` in `zplFieldPrimitives` that sanitises user input before patching. Default sanitiser is the new `stripUnsafeChars` helper, backed by a module-level g-flagged regex (`setupScriptUnsafeCharGlobalRegex`) so the regex isn't reallocated per keystroke. The primitive routes IME composition through real `compositionstart`/`compositionend` events instead of an `instanceof InputEvent` probe (cross-browser flake) and resets the composition flag on `blur` so a focus-loss mid-composition doesn't leave it stuck. Sanitises on `compositionend` as a safety net for browsers that skip the trailing `input` event (older Safari, some Android IMEs). Trade-off: the caret jumps to end after a mid-string strip (acceptable vs. the caret-math gymnastics the first cut invited). - Hoists the cross-field cascade out of the slice into a pure `normalizeMaintenanceTypes(merged, patch)` helper in `types/PrinterProfile`. Direction follows which side the patch object touches, so a future `^MA`-only-via-import or a message-side edit each get their cascade for free. - Migrates `printerName`, `printerDescription`, `encodingTable`, and the maintenance message field to `SafeStringInput`. Migrates the alert `print`/`units` selects and the message-text row to `ZplSubField` so the label-to-input `htmlFor` link is consistent with the rest of the modal. - Drops the `?? 0` / `?? 1` collapse in the BoundedIntControl onChange handlers. `readBoundedInt` lets sub-min positives through for asymmetric typing; the schema requires `>= min`, so the frequency cell gates the commit. Threshold (`min=0`) needs no gate. 1528 tests green.
1 parent 4817177 commit a6da71d

6 files changed

Lines changed: 240 additions & 158 deletions

File tree

‎src/components/PrinterSettings/EncodingAndLanguageTab.tsx‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { useId } from "react";
22
import { useT } from "../../lib/useT";
33
import { useLabelStore } from "../../store/labelStore";
4-
import { inputCls } from "../ui/formStyles";
54
import { PRINTER_LOCALE_VALUES, ZPL_MODE_VALUES, isPrinterLocale, isZplMode, type PrinterLocale, type ZplMode } from "../../types/PrinterProfile";
65
import {
6+
SafeStringInput,
77
ZplCommandLabel,
88
ZplEnumSelect,
99
ZplField,
@@ -67,15 +67,12 @@ export function EncodingAndLanguageTab() {
6767
conventions vary. */}
6868
<ZplField>
6969
<ZplCommandLabel text={loc.encodingTable} command="^SE" htmlFor={encodingId} />
70-
<input
70+
<SafeStringInput
7171
id={encodingId}
72-
type="text"
73-
className={`${inputCls} invalid:border-warning`}
72+
className="invalid:border-warning"
7473
pattern={SE_PATH_PATTERN}
7574
value={profile.encodingTable ?? ""}
76-
onChange={(e) =>
77-
patchPrinterProfile({ encodingTable: e.target.value || undefined })
78-
}
75+
onChange={(v) => patchPrinterProfile({ encodingTable: v || undefined })}
7976
/>
8077
<span className="font-mono text-[10px] text-muted/70 normal-case tracking-normal">
8178
{loc.encodingTableHint}

‎src/components/PrinterSettings/IdentityTab.tsx‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
isConfigUpdateAction,
1111
} from "../../types/PrinterProfile";
1212
import {
13+
SafeStringInput,
1314
ZplCommandLabel,
1415
ZplEnumSelect,
1516
ZplField,
@@ -64,30 +65,22 @@ export function IdentityTab() {
6465
MediaFeedTab). */}
6566
<ZplField>
6667
<ZplCommandLabel text={loc.printerName} command="^KN" htmlFor={nameId} />
67-
<input
68+
<SafeStringInput
6869
id={nameId}
69-
type="text"
7070
maxLength={PRINTER_NAME_MAX_LEN}
71-
className={inputCls}
7271
value={profile.printerName ?? ""}
73-
onChange={(e) =>
74-
patchPrinterProfile({ printerName: e.target.value || undefined })
75-
}
72+
onChange={(v) => patchPrinterProfile({ printerName: v || undefined })}
7673
/>
7774
<span className={`${labelCls} normal-case tracking-normal text-muted/70`}>
7875
{loc.printerNameHint}
7976
</span>
8077

8178
<ZplSubField label={loc.printerDescription}>
8279
{(id) => (
83-
<input
80+
<SafeStringInput
8481
id={id}
85-
type="text"
86-
className={inputCls}
8782
value={profile.printerDescription ?? ""}
88-
onChange={(e) =>
89-
patchPrinterProfile({ printerDescription: e.target.value || undefined })
90-
}
83+
onChange={(v) => patchPrinterProfile({ printerDescription: v || undefined })}
9184
/>
9285
)}
9386
</ZplSubField>

‎src/components/PrinterSettings/MaintenanceTab.tsx‎

Lines changed: 107 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,13 @@ import {
1717
type MaintenanceAlertUnit,
1818
} from "../../types/PrinterProfile";
1919
import {
20+
BoundedIntControl,
21+
SafeStringInput,
2022
ZplBoundedIntInput,
2123
ZplCommandLabel,
2224
ZplEnumSelect,
2325
ZplField,
26+
ZplSubField,
2427
} from "./zplFieldPrimitives";
2528

2629
type LocMaintenance = ReturnType<typeof useT>["printerSettings"]["maintenance"];
@@ -46,18 +49,12 @@ export function MaintenanceTab() {
4649
const alert = profile.maintenanceAlert;
4750
const message = profile.maintenanceMessage;
4851

49-
// Centralised mutation: any change to maintenanceAlert.type also
50-
// cascades into maintenanceMessage.type so the cross-field invariant
51-
// (superRefine in PrinterProfile.ts) never trips and a silent patch
52-
// rollback never strands the UI.
52+
// Cascade between maintenanceAlert.type and maintenanceMessage.type
53+
// lives in patchPrinterProfile so UI call sites don't carry the
54+
// invariant; this helper only owns the per-field merge.
5355
const updateAlert = <K extends keyof MaintenanceAlert>(key: K, value: MaintenanceAlert[K]) => {
5456
if (!alert) return;
55-
const nextAlert = { ...alert, [key]: value };
56-
const next: Partial<PrinterProfile> = { maintenanceAlert: nextAlert };
57-
if (key === "type" && message) {
58-
next.maintenanceMessage = { ...message, type: nextAlert.type };
59-
}
60-
patchPrinterProfile(next);
57+
patchPrinterProfile({ maintenanceAlert: { ...alert, [key]: value } });
6158
};
6259

6360
return (
@@ -78,18 +75,14 @@ export function MaintenanceTab() {
7875
return;
7976
}
8077
if (!isMaintenanceAlertType(raw)) return;
81-
if (alert) {
82-
updateAlert("type", raw);
83-
return;
84-
}
85-
// First-time alert creation: cascade into an
86-
// existing typed-ahead message so the cross-field
87-
// invariant holds atomically.
88-
const next: Partial<PrinterProfile> = {
89-
maintenanceAlert: { ...MAINTENANCE_ALERT_DEFAULTS, type: raw },
90-
};
91-
if (message) next.maintenanceMessage = { ...message, type: raw };
92-
patchPrinterProfile(next);
78+
// First-time creation seeds defaults; subsequent edits
79+
// re-spread the alert. The cross-field type cascade
80+
// is enforced by patchPrinterProfile.
81+
patchPrinterProfile({
82+
maintenanceAlert: alert
83+
? { ...alert, type: raw }
84+
: { ...MAINTENANCE_ALERT_DEFAULTS, type: raw },
85+
});
9386
}}
9487
>
9588
<option value="">{t.printerSettings.defaultOption}</option>
@@ -98,90 +91,107 @@ export function MaintenanceTab() {
9891
))}
9992
</select>
10093
</div>
101-
<div className="flex flex-col gap-1">
102-
<label className={labelCls}>{loc.alertPrint}</label>
103-
<select
104-
className={inputCls}
105-
value={alert?.print ?? MAINTENANCE_ALERT_DEFAULTS.print}
106-
onChange={(e) => {
107-
const raw = e.target.value;
108-
if (isMaintenanceAlertPrint(raw)) updateAlert("print", raw);
109-
}}
110-
disabled={!alert}
111-
>
112-
{MAINTENANCE_ALERT_PRINT_VALUES.map((v) => (
113-
<option key={v} value={v}>{v === "Y" ? loc.alertPrintY : loc.alertPrintN}</option>
114-
))}
115-
</select>
116-
</div>
94+
<ZplSubField label={loc.alertPrint}>
95+
{(id) => (
96+
<select
97+
id={id}
98+
className={inputCls}
99+
value={alert?.print ?? MAINTENANCE_ALERT_DEFAULTS.print}
100+
onChange={(e) => {
101+
const raw = e.target.value;
102+
if (isMaintenanceAlertPrint(raw)) updateAlert("print", raw);
103+
}}
104+
disabled={!alert}
105+
>
106+
{MAINTENANCE_ALERT_PRINT_VALUES.map((v) => (
107+
<option key={v} value={v}>{v === "Y" ? loc.alertPrintY : loc.alertPrintN}</option>
108+
))}
109+
</select>
110+
)}
111+
</ZplSubField>
117112
</div>
118113
<div className="grid grid-cols-3 gap-2">
119-
<BoundedIntCell
120-
label={loc.alertThreshold}
121-
min={0}
122-
max={MAINTENANCE_DISTANCE_MAX}
123-
value={alert?.threshold}
124-
onChange={(threshold) => {
125-
if (threshold !== undefined) updateAlert("threshold", threshold);
126-
}}
127-
disabled={!alert}
128-
/>
129-
<BoundedIntCell
130-
label={loc.alertFrequency}
131-
min={1}
132-
max={MAINTENANCE_DISTANCE_MAX}
133-
value={alert?.frequency}
134-
onChange={(frequency) => {
135-
if (frequency !== undefined) updateAlert("frequency", frequency);
136-
}}
137-
disabled={!alert}
138-
/>
139-
<div className="flex flex-col gap-1">
140-
<label className={labelCls}>{loc.alertUnits}</label>
141-
<select
142-
className={inputCls}
143-
value={alert?.units ?? MAINTENANCE_ALERT_DEFAULTS.units}
144-
onChange={(e) => {
145-
const raw = e.target.value;
146-
if (isMaintenanceAlertUnit(raw)) updateAlert("units", raw);
147-
}}
148-
disabled={!alert}
149-
>
150-
{MAINTENANCE_ALERT_UNITS.map((v) => (
151-
<option key={v} value={v}>{loc[ALERT_UNIT_LABEL_KEYS[v]]}</option>
152-
))}
153-
</select>
154-
</div>
114+
<ZplSubField label={loc.alertThreshold}>
115+
{(id) => (
116+
<BoundedIntControl
117+
id={id}
118+
min={0}
119+
max={MAINTENANCE_DISTANCE_MAX}
120+
value={alert?.threshold}
121+
disabled={!alert}
122+
onChange={(threshold) => {
123+
// Drop undefined (mid-typing empty); blur clamps stale state.
124+
if (threshold !== undefined) updateAlert("threshold", threshold);
125+
}}
126+
/>
127+
)}
128+
</ZplSubField>
129+
<ZplSubField label={loc.alertFrequency}>
130+
{(id) => (
131+
<BoundedIntControl
132+
id={id}
133+
min={1}
134+
max={MAINTENANCE_DISTANCE_MAX}
135+
value={alert?.frequency}
136+
disabled={!alert}
137+
onChange={(frequency) => {
138+
// readBoundedInt lets sub-min positives through so a
139+
// user typing "1" toward "12" survives a min=2 field;
140+
// schema requires >= min, so gate the commit here.
141+
if (frequency !== undefined && frequency >= 1) {
142+
updateAlert("frequency", frequency);
143+
}
144+
}}
145+
/>
146+
)}
147+
</ZplSubField>
148+
<ZplSubField label={loc.alertUnits}>
149+
{(id) => (
150+
<select
151+
id={id}
152+
className={inputCls}
153+
value={alert?.units ?? MAINTENANCE_ALERT_DEFAULTS.units}
154+
onChange={(e) => {
155+
const raw = e.target.value;
156+
if (isMaintenanceAlertUnit(raw)) updateAlert("units", raw);
157+
}}
158+
disabled={!alert}
159+
>
160+
{MAINTENANCE_ALERT_UNITS.map((v) => (
161+
<option key={v} value={v}>{loc[ALERT_UNIT_LABEL_KEYS[v]]}</option>
162+
))}
163+
</select>
164+
)}
165+
</ZplSubField>
155166
</div>
156167
</div>
157168
</ZplField>
158169

159170
<ZplField>
160171
<ZplCommandLabel text={loc.maintenanceMessageHeading} command="^MI" />
161172
<div className="flex flex-col gap-2 pl-2 border-l border-border">
162-
<div className="flex flex-col gap-1">
163-
<label className={labelCls}>{loc.messageText}</label>
164-
<input
165-
type="text"
166-
className={inputCls}
167-
maxLength={MAINTENANCE_MESSAGE_MAX_LEN}
168-
value={message?.text ?? ""}
169-
onChange={(e) => {
170-
const text = e.target.value;
171-
if (!text) {
172-
patchPrinterProfile({ maintenanceMessage: undefined });
173-
return;
174-
}
175-
// Type sticks to alert.type when an alert exists; otherwise
176-
// falls back to the default so superRefine stays satisfied.
177-
const type = message?.type ?? alert?.type ?? MAINTENANCE_ALERT_DEFAULTS.type;
178-
patchPrinterProfile({ maintenanceMessage: { type, text } });
179-
}}
180-
/>
181-
<span className={`${labelCls} normal-case tracking-normal text-muted/70`}>
182-
{loc.messageTextHint} ({MAINTENANCE_MESSAGE_MAX_LEN})
183-
</span>
184-
</div>
173+
<ZplSubField label={loc.messageText}>
174+
{(id) => (
175+
<SafeStringInput
176+
id={id}
177+
maxLength={MAINTENANCE_MESSAGE_MAX_LEN}
178+
value={message?.text ?? ""}
179+
onChange={(text) => {
180+
if (!text) {
181+
patchPrinterProfile({ maintenanceMessage: undefined });
182+
return;
183+
}
184+
// Type sticks to alert.type when an alert exists; otherwise
185+
// falls back to the default so superRefine stays satisfied.
186+
const type = message?.type ?? alert?.type ?? MAINTENANCE_ALERT_DEFAULTS.type;
187+
patchPrinterProfile({ maintenanceMessage: { type, text } });
188+
}}
189+
/>
190+
)}
191+
</ZplSubField>
192+
<span className={`${labelCls} normal-case tracking-normal text-muted/70`}>
193+
{loc.messageTextHint} ({MAINTENANCE_MESSAGE_MAX_LEN})
194+
</span>
185195
</div>
186196
</ZplField>
187197

@@ -228,39 +238,3 @@ export function MaintenanceTab() {
228238
);
229239
}
230240

231-
function BoundedIntCell({
232-
label,
233-
min,
234-
max,
235-
value,
236-
onChange,
237-
disabled,
238-
}: {
239-
label: string;
240-
min: number;
241-
max: number;
242-
value: number | undefined;
243-
onChange: (next: number | undefined) => void;
244-
disabled?: boolean;
245-
}) {
246-
return (
247-
<div className="flex flex-col gap-1">
248-
<label className={labelCls}>{label}</label>
249-
<input
250-
type="number"
251-
className={inputCls}
252-
min={min}
253-
max={max}
254-
value={value ?? ""}
255-
disabled={disabled}
256-
onChange={(e) => {
257-
const raw = e.target.value;
258-
if (raw === "") return;
259-
const n = Number.parseInt(raw, 10);
260-
if (Number.isNaN(n)) return;
261-
onChange(Math.max(min, Math.min(max, n)));
262-
}}
263-
/>
264-
</div>
265-
);
266-
}

0 commit comments

Comments
 (0)