Task Form Embed PR Review

Long forms cut off in a task's Work view: review notes (PR #198)

✅ Done - merged for 3.13.0 on 21 October 2026, as @Dr4inder's own commit (85e5a1a) followed by a review commit of mine (d286bf5). This page is the record of the review.

Written for @Dr4inder, who found and fixed it, and for anyone - or any AI coding assistant - contributing to FreeITSM next. Same shape as the other reviews: your code does X, the problem is Y, the approach now is Z, then a verdict: keep, change or leave out. Lessons that keep coming back are collected in Contributing.

PR	What	Verdict
#198	A form opened inside a task (the Work view, #190) stayed at the loading card's height, hiding its later questions and the Submit button	Merge with fixes (small - the fix is unchanged; the test's driver was rewritten)

---

The bug, in one paragraph

A task's form is a forms/fill.php page in an iframe. The page tells the task window how tall it is, and the task window sizes the frame to match. It measured and watched <body> - but in a frame, the body is as tall as the frame itself, so it never changed when the form's fields arrived after loading. The frame stayed at the loading card's height, and overflow: hidden meant there was no way to scroll to the rest. Measuring document.documentElement.scrollHeight instead would have made it grow but never shrink, because the page is never shorter than the frame it is in.

Keep - all of the fix

• Measure and watch the content, not the page. .fill-content is the form itself, so it grows when fields load or appear and shrinks when conditional fields hide. I checked that nothing the form shows is drawn outside it.
• A way out if the message never arrives. The embedded body can scroll now, so even if the size message is lost, every field and the Submit button can still be reached.
• The border counted. The task window adds the frame's own border, so the content is not cut off by two pixels.
• A TRAP: comment at the exact line, saying why the body is the wrong thing to watch. That is the house style, and it is what stops the next person "simplifying" it back.
• The test. It lifts the real styles and the real message handlers out of the two files rather than copying them, so it tests what ships. It covers late fields, conditional fields growing and shrinking, a narrow layout, the no-ResizeObserver fallback, and scrolling with no reporter at all. It touches no database and no settings. And it fails on the old code - 8 of 9 checks - which is what makes it worth having.

I also opened a real task with a form on my own install and checked it: the whole form shows, the frame matches the content exactly, and Submit and Save draft are inside it.

Change

#	Finding	Verdict
1	The test needed Node.js 22	Change
2	Conflict with main on the script versions	Change

1. Node.js. Your test drove the browser with a Node.js script over Chrome's DevTools socket, because the checks wait on real animation frames, which a --dump-dom --virtual-time-budget run never advances (I tried; it stalls at "RUNNING", exactly as your comment warns). The problem: every other test here runs with php tests/<name>.php and nothing else, and Node is not on every machine these are run on - including mine. A test that cannot run is a test that will not be run. Now: the same browser, the same real-time run, the same DevTools call (Runtime.evaluate waiting for the result) - but from PHP. cdpEvaluate() at the top of tests/task-form-embed.php is a small WebSocket client: the handshake, a masked frame out, frames back until the answer arrives. Everything else in your test is as you wrote it. Results: 9 passed; against the old code, 8 of 9 fail - the same as you reported.

2. Script versions. Your PR moved tasks.js to ?v=65. The problem: main had already used v=65 (and moved mobile.js on) since you branched, so the PR conflicted and the number would not have reached anyone's browser cache. Now: rebased, with tasks.js?v=66. Nothing for you to have done differently - this is just what happens when two changes land on the same line.

For next time

• Browser tests: PHP and Chrome only. If a test needs a real browser, drive it with cdpEvaluate() from tests/task-form-embed.php - copy it. Now in Contributing.
• Everything else here is the standard to copy: a small fix in the right place, a TRAP: comment, a fallback, and a test with a negative control.

See also

• Contributing · REST custom fields, evidence and inventory - review (#195-#197)