fix(export): keep injected attrs out of the WHP-FORM marker comment
injectAttrs() skipped leading <style> blocks and then inserted the responsive/visibility/animation attrs before the first '>'. Relay forms (ContactForm, SubscribeForm, FormContainer with a recipient) start their toHtml with `<!--WHP-FORM ...-->`, so the attrs landed inside the marker (`thankyou=""-- data-wsb-r="...">`): the comment was left unterminated, Preview swallowed the form, and the publish step refused the whole site. injectAttrs now skips any leading run of whitespace, <style> blocks and <!--...--> comments (any order, repeated) and targets the first real element; a string that is only such a run is returned unchanged. The marker wire format is untouched. Tests port the backend's strict marker regex from FormRelayRewrite.php and export each relay form with a recipient + styleMobile/styleTablet + hideOnMobile + animation through exportBodyHtml. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
ff826a03c4
commit
7fe47160e9
2 files changed
+109
-10
No files matched your search
@@ -787,3 +787,94 @@ describe('#54: canonical link in <head>', () => {
|
||||
expect(buildSeoMeta({ canonicalUrl: '/relative' }, 'P')).not.toContain('canonical');
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Finding 6: relay forms (ContactForm / SubscribeForm / FormContainer with a
|
||||
* recipient) start their toHtml with a `<!--WHP-FORM ...-->` marker comment.
|
||||
* injectAttrs used to insert responsive/visibility/animation attrs before the
|
||||
* FIRST `>` -- the end of that comment -- which broke the marker
|
||||
* (`thankyou=""-- data-wsb-r="...">`), swallowed the <form> into an
|
||||
* unterminated comment in Preview, and made the backend refuse the publish.
|
||||
*/
|
||||
describe('relay-form WHP-FORM marker survives injected attrs (finding 6)', () => {
|
||||
// Ported verbatim from web-files/libs/FormRelayRewrite.php
|
||||
// fs_rewrite_contact_forms() -- the strict marker regex the publish step uses.
|
||||
const BACKEND_MARKER_RE =
|
||||
/<!--WHP-FORM id="([^"]+)"((?: [a-z]+="[^"]*")*) recipient="([^"]*)" thankyou="([^"]*)"-->/g;
|
||||
|
||||
const node = (type: string, props: Record<string, unknown>, nodes: string[] = [], parent?: string) => ({
|
||||
type: { resolvedName: type },
|
||||
isCanvas: type === 'Container' || type === 'FormContainer',
|
||||
props,
|
||||
displayName: type,
|
||||
custom: {},
|
||||
hidden: false,
|
||||
nodes,
|
||||
linkedNodes: {},
|
||||
...(parent ? { parent } : {}),
|
||||
});
|
||||
|
||||
const overrides = {
|
||||
recipientEmail: 'owner@example.com',
|
||||
thankYouUrl: '/thanks',
|
||||
styleMobile: { padding: '8px' },
|
||||
styleTablet: { margin: '4px' },
|
||||
hideOnMobile: true,
|
||||
animation: 'fade-in',
|
||||
animationDelay: '0.3s',
|
||||
};
|
||||
|
||||
const formState = (type: string, extra: Record<string, unknown> = {}) =>
|
||||
JSON.stringify({
|
||||
ROOT: node('Container', { tag: 'div', style: {} }, ['form1']),
|
||||
form1: node(type, { ...overrides, ...extra }, [], 'ROOT'),
|
||||
});
|
||||
|
||||
const cases: Array<[string, string]> = [
|
||||
['ContactForm', 'form'],
|
||||
['SubscribeForm', 'div'],
|
||||
['FormContainer', 'form'],
|
||||
];
|
||||
|
||||
test.each(cases)('%s: marker intact + attrs land on the first real <%s>', (type, tag) => {
|
||||
const { html } = exportBodyHtml(formState(type));
|
||||
const markers = [...html.matchAll(BACKEND_MARKER_RE)];
|
||||
expect(markers).toHaveLength(1);
|
||||
const [marker, , , recipient, thankyou] = markers[0];
|
||||
expect(recipient).toBe('owner@example.com');
|
||||
expect(thankyou).toBe('/thanks');
|
||||
expect(marker).not.toContain('data-');
|
||||
// The element immediately after the marker carries every injected attr.
|
||||
const after = html.slice(html.indexOf(marker) + marker.length);
|
||||
const openTag = after.match(/^<([a-z]+)\b[^>]*>/);
|
||||
expect(openTag, `no element right after marker: ${after.slice(0, 80)}`).not.toBeNull();
|
||||
expect(openTag![1]).toBe(tag);
|
||||
expect(openTag![0]).toContain('data-wsb-r="b-form1"');
|
||||
expect(openTag![0]).toContain('data-hide-mobile');
|
||||
expect(openTag![0]).toContain('data-animation="fade-in"');
|
||||
expect(openTag![0]).toContain('data-animation-delay="0.3s"');
|
||||
// Exactly one comment terminator for the marker -- nothing swallowed.
|
||||
expect(html.match(/<!--/g)!.length).toBe(html.match(/-->/g)!.length);
|
||||
});
|
||||
});
|
||||
|
||||
describe('injectAttrs skips leading comments / style blocks / whitespace (finding 6)', () => {
|
||||
test('leading comment', () => {
|
||||
expect(injectAttrs('<!--x-->\n<form a="1">y</form>', ' data-z')).toBe('<!--x-->\n<form a="1" data-z>y</form>');
|
||||
});
|
||||
test('mixed comment + style + whitespace, any order, repeated', () => {
|
||||
const html = ' <style>.a{}</style> <!--m1--><!-- m2 --> <style>.b{}</style>\n<div>z</div>';
|
||||
expect(injectAttrs(html, ' data-z')).toBe(
|
||||
' <style>.a{}</style> <!--m1--><!-- m2 --> <style>.b{}</style>\n<div data-z>z</div>',
|
||||
);
|
||||
});
|
||||
test('comment containing ">" is skipped as a unit', () => {
|
||||
expect(injectAttrs('<!-- a > b --><p>t</p>', ' data-z')).toBe('<!-- a > b --><p data-z>t</p>');
|
||||
});
|
||||
test('bare comment with nothing after is left alone', () => {
|
||||
expect(injectAttrs('<!--only-->', ' data-z')).toBe('<!--only-->');
|
||||
});
|
||||
test('void tag after a comment still gets attrs before the slash', () => {
|
||||
expect(injectAttrs('<!--c--><img src="x" />', ' data-z')).toBe('<!--c--><img src="x" data-z/>');
|
||||
});
|
||||
});
|
||||
@@ -70,13 +70,21 @@ function buildDataAttrs(props: Record<string, any>): string {
|
||||
* the attrs before it instead, yielding well-formed `<img ... data-animation="..."/>`.
|
||||
* Non-void tags (no trailing `/`) are unaffected.
|
||||
*
|
||||
* Leading `<style>…</style>` elements (components such as ButtonLink and
|
||||
* ColumnLayout emit a scoped style block before their element) are skipped,
|
||||
* so the attrs land on the component's real element, not on the `<style>`.
|
||||
* Leading whitespace, `<style>…</style>` elements (components such as
|
||||
* ButtonLink and ColumnLayout emit a scoped style block before their element)
|
||||
* and `<!--…-->` comments (relay forms -- ContactForm, SubscribeForm,
|
||||
* FormContainer -- emit the `<!--WHP-FORM …-->` marker before their element)
|
||||
* are skipped, in any order and repeated, so the attrs land on the
|
||||
* component's first REAL element. Injecting into the marker comment would
|
||||
* break it (`…thankyou=""-- data-wsb-r="…">`): the browser swallows the form
|
||||
* into an unterminated comment, and the publish step refuses the page.
|
||||
*/
|
||||
export function injectAttrs(html: string, attrs: string): string {
|
||||
if (!attrs) return html;
|
||||
const prefixLen = leadingStyleLength(html);
|
||||
const prefixLen = leadingNonElementLength(html);
|
||||
// Nothing but style blocks / comments: there is no element to carry the
|
||||
// attrs, and injecting into a comment would corrupt it -- leave it alone.
|
||||
if (prefixLen > 0 && prefixLen >= html.length) return html;
|
||||
if (prefixLen > 0) return html.slice(0, prefixLen) + injectAttrs(html.slice(prefixLen), attrs);
|
||||
// Find the first > of the opening tag and inject before it
|
||||
const idx = html.indexOf('>');
|
||||
@@ -94,18 +102,18 @@ export function injectAttrs(html: string, attrs: string): string {
|
||||
return html.slice(0, idx) + attrs + html.slice(idx);
|
||||
}
|
||||
|
||||
/** Length of any run of `<style …>…</style>` elements (plus whitespace) at the
|
||||
* very start of `html`; 0 when it does not start with one. Only skips when
|
||||
* something follows the style block(s) -- a bare style block is left alone. */
|
||||
function leadingStyleLength(html: string): number {
|
||||
const re = /^\s*<style\b[^>]*>[\s\S]*?<\/style>\s*/i;
|
||||
/** Length of any run of `<style …>…</style>` elements and `<!--…-->` comments
|
||||
* (plus whitespace), in any order, at the very start of `html`; 0 when it
|
||||
* does not start with one. Equals `html.length` when nothing follows. */
|
||||
function leadingNonElementLength(html: string): number {
|
||||
const re = /^\s*(?:<style\b[^>]*>[\s\S]*?<\/style>|<!--[\s\S]*?-->)\s*/i;
|
||||
let len = 0;
|
||||
for (;;) {
|
||||
const m = re.exec(html.slice(len));
|
||||
if (!m) break;
|
||||
len += m[0].length;
|
||||
}
|
||||
return len < html.length ? len : 0;
|
||||
return len;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in new issue
Block a user