Skip to content

Commit f3bd223

Browse files
JohnMcLearclaude
andcommitted
fix(admin): reset rejected env-pill drafts on blur, don't drop decoded braces
Address review: EnvPill now marks undecodable drafts aria-invalid and restores the applied value on blur (matching StringInput), and a default whose decoded form contains `}` is rejected/kept raw instead of having the brace silently stripped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012kA75NPq8nGRidAwhPXeCi
1 parent dec36a8 commit f3bd223

2 files changed

Lines changed: 44 additions & 5 deletions

File tree

‎admin/src/components/settings/widgets/EnvPill.tsx‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@ type Props = {
1515

1616
const sanitize = (s: string) => s.replace(/[}]/g, '');
1717

18+
// Decode an escaped default. A `}` (e.g. from `\u007d`) would terminate the
19+
// `${VAR:default}` placeholder, so treat it as invalid rather than silently
20+
// dropping it.
21+
const decodeDefault = (s: string): string | null => {
22+
const decoded = unescapeFromInput(s);
23+
return decoded === null || decoded.includes('}') ? null : decoded;
24+
};
25+
1826
const formatDisplay = (v: unknown): string => {
1927
if (v === null) return 'null';
2028
if (typeof v === 'string') return v;
@@ -27,13 +35,17 @@ export const EnvPill = ({ placeholder, path, onChange, resolvedValue }: Props) =
2735
// Normalise it through decode/escape so it matches what StringInput shows
2836
// (e.g. `https:\/\/` displays as `https://`) — see stringEscapes.ts.
2937
const rawDefault = placeholder.defaultValue ?? '';
30-
const decodedDefault = unescapeFromInput(rawDefault);
38+
const decodedDefault = decodeDefault(rawDefault);
3139
const initial = decodedDefault === null ? rawDefault : escapeForInput(decodedDefault);
3240
const [draft, setDraft] = useState(initial);
41+
const [invalid, setInvalid] = useState(false);
3342
const focused = useRef(false);
3443

3544
useEffect(() => {
36-
if (!focused.current) setDraft(initial);
45+
if (!focused.current) {
46+
setDraft(initial);
47+
setInvalid(false);
48+
}
3749
}, [initial]);
3850

3951
const id = `field-${path.join('.')}`;
@@ -64,16 +76,24 @@ export const EnvPill = ({ placeholder, path, onChange, resolvedValue }: Props) =
6476
value={draft}
6577
spellCheck={false}
6678
aria-label={t('admin_settings.env_pill.input_aria', { variable: placeholder.variable })}
79+
aria-invalid={invalid || undefined}
6780
onFocus={() => { focused.current = true; }}
68-
onBlur={() => { focused.current = false; }}
81+
onBlur={() => {
82+
focused.current = false;
83+
// Drop a rejected draft so the field never shows a value that
84+
// was not applied to the settings text.
85+
setDraft(initial);
86+
setInvalid(false);
87+
}}
6988
onChange={e => {
7089
const v = sanitize(e.target.value);
7190
setDraft(v);
7291
// The draft is in escaped form; hand the decoded value to the
7392
// JSON writer so typed `\n` is stored as `\n`, not `\\n` (#8211).
7493
// Incomplete escapes (a trailing `\`) are not propagated.
75-
const decoded = unescapeFromInput(v);
76-
if (decoded !== null) onChange(sanitize(decoded));
94+
const decoded = decodeDefault(v);
95+
setInvalid(decoded === null);
96+
if (decoded !== null) onChange(decoded);
7797
}}
7898
/>
7999
{hasResolved && !isRedacted && (

‎admin/src/components/settings/widgets/__tests__/EnvPill.test.tsx‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,22 @@ test('renders null resolved value as the string null', () => {
8484
} as any));
8585
assert.ok(html.includes('null'), `expected "null" in ${html}`);
8686
});
87+
88+
// https://github.com/ether/etherpad/issues/8211
89+
test('shows escaped defaults in their escaped form', () => {
90+
const html = wrap(React.createElement(EnvPill, {
91+
placeholder: { variable: 'DEFAULT_PAD_TEXT', defaultValue: 'Line 1\\nLine 2 https:\\/\\/x' },
92+
path: ['defaultPadText'],
93+
onChange: () => {},
94+
}));
95+
assert.ok(html.includes('value="Line 1\\nLine 2 https://x"'), html);
96+
});
97+
98+
test('keeps a default whose decoded form contains } in raw form', () => {
99+
const html = wrap(React.createElement(EnvPill, {
100+
placeholder: { variable: 'X', defaultValue: 'a\\u007db' },
101+
path: ['x'],
102+
onChange: () => {},
103+
}));
104+
assert.ok(html.includes('value="a\\u007db"'), html);
105+
});

0 commit comments

Comments
 (0)