fix(js): initialize a model added after the document is ready - #23

Merged
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:fix/init-model-after-document-ready
Aug 27, 2026
Merged

fix(js): initialize a model added after the document is ready#23
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:fix/init-model-after-document-ready

Conversation

@66Ton99

Copy link
Copy Markdown

Problem

addModel(model, onLoad) defers registration to onDocumentReady(), and that helper only adds a DOMContentLoaded listener:

this.onDocumentReady=function(callback){varaddListener=document.addEventListener||document.attachEvent;
...
addListener.call(document,eventName,function(callee){ ... callback();},false)};

When a page fetches a rendered form after the initial load and injects it — a single page CRUD, a modal that loads its form, a wizard step — the addModel() call the fragment carries runs when DOMContentLoaded has already been dispatched. The listener is never called, so the form is registered nowhere and keeps no validator at all.

The default of init_js_validation(form) is onLoad = true, so this is the path a fragment hits unless the application knows to pass false. onLoad = false is affected too, in a smaller way: it registers the model right away but still defers disableNativeValidationUi() through the same helper, so with html5_validation: true an injected form keeps the native bubble that stops the submit event this library listens to.

Change

onDocumentReady() runs the callback immediately once document.readyState is past loading, and still waits for the event while the document is being parsed, so the initial page load is unchanged — a model printed inline is still initialized after the document is ready, which is what lets js_validator_config() appear further down the page.

Tests

Two Jest tests in SvarohJsFormValidator.test.js: a model added while the document is ready registers its form, and a model added while document.readyState is loading still waits for the event. The first fails on main.

npx jest — 610 tests pass (608 before). PHP checks are untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

🤖 Generated with Claude Code

Ton Sharpand others added 2 commits August 27, 2026 22:47
"addModel" defers registration to "onDocumentReady", which only added a
"DOMContentLoaded" listener. A form fragment that an application fetches
and injects after the initial load runs its "addModel" call when that
event has already been dispatched, so the listener was never called and
the form kept no validator at all.
The callback now runs immediately once "document.readyState" is past
"loading", and still waits for the event while the document is parsing.
"onDocumentReady" ran the callback for every state past "loading", but
"interactive" means the document is parsed while its deferred scripts,
and the "js_validator_config()" one of them may carry, still run before
"DOMContentLoaded". Only "complete" stands for an event that is really
gone, so the call a model added with "onLoad = false" defers keeps
waiting for the configuration it turns the native UI off with. The same
condition is what old IE needs, where "interactive" does not mean the
document can be walked yet.
The listener the other branch adds is removable again. It passed the
event it received to "removeEventListener", which identifies no
listener, so every call left one behind, and the fallback ran its
callback on every "readystatechange" instead of the last one.
A single page application that swaps a rendered form for a new one left
the element of the node it removed in "formInstances", and every
reopened modal or revisited wizard step added one more. A render that is
no longer in the document is now dropped when the same form is
initialized again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@66Ton99

Copy link
Copy Markdown
Author

Follow-up from review — b7c40c6

A review of the first commit turned up three things the change either introduced or left standing.

interactive is not a document that is done

The immediate branch fired for every state past loading. But interactive means the document is parsed while its deferred scripts still run, and DOMContentLoaded has not been dispatched yet.

That matters because onDocumentReady() does double duty in addModel(). The second call, on the onLoad = false branch, is not a DOM-readiness check at all — it is an ordering barrier, holding disableNativeValidationUi() until a js_validator_config() printed further down the document has set config. A defer or type="module" script would have lost that barrier and left the form without novalidate under html5_validation: true.

The condition is now 'complete' === document.readyState, the only state where the event really is gone. It happens to be what old IE needs too: there interactive does not mean the tree can be walked yet, which is why jQuery pairs its check with documentElement.doScroll.

The listener was never removed

Pre-existing, but the change lives in that function:

addListener.call(document,eventName,function(callee){removeListener.call(this,eventName,callee,false);

callee is the event, not the handler, so removeEventListener() matched nothing and every call left a listener behind. attachEvent passes no argument at all and binds this to window, so the fallback path was broken twice over and ran its callback on every readystatechange instead of the last one. The handler now names itself and detaches from document.

Reopening a modal piled up instances

The scenario the first commit unlocks is a form injected more than once. registerForm() only replaced an instance when domNode was identical, so every re-injection appended one more element pointing at a node that had been taken out of the document, and getFormInstances() handed those out. Instances whose node is no longer in the document are dropped when the same form is initialized again, guarded on document.contains so a browser that cannot answer keeps the old behaviour.

Tests

withReadyState() drives the document state, since jsdom reports complete for a whole run. The existing skips a model without a DOM node on the deferred branch too had quietly stopped deferring — it registered synchronously and its dispatchEvent() was a no-op — so it stubs loading again and asserts it.

Five tests added, each verified to fail on a revert of its own fix and nothing else:

revertedfails
the readyState conditionstill waits for the document while its deferred scripts run, turns the native UI off with a configuration a deferred script sets
the named handlerleaves no listener behind once the document is ready
dropping detached instancesforgets the instance of a render that was taken out of the document

Docs

2_3.md gains two subsections: one on forms loaded after the page — including that innerHTML does not execute a <script> tag it inserts, so a fragment dropped that way is rendered and never initialized — and one holding the existing onLoad = false example. 3_20.md notes the new getFormInstances() behaviour.

Verification

Full suite in the Nix shell, PHP 8.5.6 / Node 24.16.0:

  • composer test — 95 tests, 268 assertions
  • composer phpstan — no errors
  • npm run test:unit — 615 tests
  • npm run test:coverage — 94.67% line coverage, threshold 80%
  • Cypress e2e — 24/24

The e2e run earns its place here: jsdom cannot tell interactive from complete, so a real browser is the only place the ordering claim is exercised end to end.

This supersedes the test count and the "PHP checks were not run" note in the description above.

@66Ton99
66Ton99 merged commit 35440f0 into Svaroh:mainAug 27, 2026
6 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@66Ton99
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(js): initialize a model added after the document is ready - #23

Merged
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:fix/init-model-after-document-ready
Aug 27, 2026
Merged

fix(js): initialize a model added after the document is ready#23
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:fix/init-model-after-document-ready

Conversation

@66Ton99

Copy link
Copy Markdown

Problem

addModel(model, onLoad) defers registration to onDocumentReady(), and that helper only adds a DOMContentLoaded listener:

this.onDocumentReady=function(callback){varaddListener=document.addEventListener||document.attachEvent;
...
addListener.call(document,eventName,function(callee){ ... callback();},false)};

When a page fetches a rendered form after the initial load and injects it — a single page CRUD, a modal that loads its form, a wizard step — the addModel() call the fragment carries runs when DOMContentLoaded has already been dispatched. The listener is never called, so the form is registered nowhere and keeps no validator at all.

The default of init_js_validation(form) is onLoad = true, so this is the path a fragment hits unless the application knows to pass false. onLoad = false is affected too, in a smaller way: it registers the model right away but still defers disableNativeValidationUi() through the same helper, so with html5_validation: true an injected form keeps the native bubble that stops the submit event this library listens to.

Change

onDocumentReady() runs the callback immediately once document.readyState is past loading, and still waits for the event while the document is being parsed, so the initial page load is unchanged — a model printed inline is still initialized after the document is ready, which is what lets js_validator_config() appear further down the page.

Tests

Two Jest tests in SvarohJsFormValidator.test.js: a model added while the document is ready registers its form, and a model added while document.readyState is loading still waits for the event. The first fails on main.

npx jest — 610 tests pass (608 before). PHP checks are untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

🤖 Generated with Claude Code

Ton Sharpand others added 2 commits August 27, 2026 22:47
"addModel" defers registration to "onDocumentReady", which only added a
"DOMContentLoaded" listener. A form fragment that an application fetches
and injects after the initial load runs its "addModel" call when that
event has already been dispatched, so the listener was never called and
the form kept no validator at all.
The callback now runs immediately once "document.readyState" is past
"loading", and still waits for the event while the document is parsing.
"onDocumentReady" ran the callback for every state past "loading", but
"interactive" means the document is parsed while its deferred scripts,
and the "js_validator_config()" one of them may carry, still run before
"DOMContentLoaded". Only "complete" stands for an event that is really
gone, so the call a model added with "onLoad = false" defers keeps
waiting for the configuration it turns the native UI off with. The same
condition is what old IE needs, where "interactive" does not mean the
document can be walked yet.
The listener the other branch adds is removable again. It passed the
event it received to "removeEventListener", which identifies no
listener, so every call left one behind, and the fallback ran its
callback on every "readystatechange" instead of the last one.
A single page application that swaps a rendered form for a new one left
the element of the node it removed in "formInstances", and every
reopened modal or revisited wizard step added one more. A render that is
no longer in the document is now dropped when the same form is
initialized again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@66Ton99

Copy link
Copy Markdown
Author

Follow-up from review — b7c40c6

A review of the first commit turned up three things the change either introduced or left standing.

interactive is not a document that is done

The immediate branch fired for every state past loading. But interactive means the document is parsed while its deferred scripts still run, and DOMContentLoaded has not been dispatched yet.

That matters because onDocumentReady() does double duty in addModel(). The second call, on the onLoad = false branch, is not a DOM-readiness check at all — it is an ordering barrier, holding disableNativeValidationUi() until a js_validator_config() printed further down the document has set config. A defer or type="module" script would have lost that barrier and left the form without novalidate under html5_validation: true.

The condition is now 'complete' === document.readyState, the only state where the event really is gone. It happens to be what old IE needs too: there interactive does not mean the tree can be walked yet, which is why jQuery pairs its check with documentElement.doScroll.

The listener was never removed

Pre-existing, but the change lives in that function:

addListener.call(document,eventName,function(callee){removeListener.call(this,eventName,callee,false);

callee is the event, not the handler, so removeEventListener() matched nothing and every call left a listener behind. attachEvent passes no argument at all and binds this to window, so the fallback path was broken twice over and ran its callback on every readystatechange instead of the last one. The handler now names itself and detaches from document.

Reopening a modal piled up instances

The scenario the first commit unlocks is a form injected more than once. registerForm() only replaced an instance when domNode was identical, so every re-injection appended one more element pointing at a node that had been taken out of the document, and getFormInstances() handed those out. Instances whose node is no longer in the document are dropped when the same form is initialized again, guarded on document.contains so a browser that cannot answer keeps the old behaviour.

Tests

withReadyState() drives the document state, since jsdom reports complete for a whole run. The existing skips a model without a DOM node on the deferred branch too had quietly stopped deferring — it registered synchronously and its dispatchEvent() was a no-op — so it stubs loading again and asserts it.

Five tests added, each verified to fail on a revert of its own fix and nothing else:

revertedfails
the readyState conditionstill waits for the document while its deferred scripts run, turns the native UI off with a configuration a deferred script sets
the named handlerleaves no listener behind once the document is ready
dropping detached instancesforgets the instance of a render that was taken out of the document

Docs

2_3.md gains two subsections: one on forms loaded after the page — including that innerHTML does not execute a <script> tag it inserts, so a fragment dropped that way is rendered and never initialized — and one holding the existing onLoad = false example. 3_20.md notes the new getFormInstances() behaviour.

Verification

Full suite in the Nix shell, PHP 8.5.6 / Node 24.16.0:

  • composer test — 95 tests, 268 assertions
  • composer phpstan — no errors
  • npm run test:unit — 615 tests
  • npm run test:coverage — 94.67% line coverage, threshold 80%
  • Cypress e2e — 24/24

The e2e run earns its place here: jsdom cannot tell interactive from complete, so a real browser is the only place the ordering claim is exercised end to end.

This supersedes the test count and the "PHP checks were not run" note in the description above.

@66Ton99
66Ton99 merged commit 35440f0 into Svaroh:mainAug 27, 2026
6 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@66Ton99
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(js): initialize a model added after the document is ready - #23

Merged
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:fix/init-model-after-document-ready
Aug 27, 2026
Merged

fix(js): initialize a model added after the document is ready#23
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:fix/init-model-after-document-ready

Conversation

@66Ton99

Copy link
Copy Markdown

Problem

addModel(model, onLoad) defers registration to onDocumentReady(), and that helper only adds a DOMContentLoaded listener:

this.onDocumentReady=function(callback){varaddListener=document.addEventListener||document.attachEvent;
...
addListener.call(document,eventName,function(callee){ ... callback();},false)};

When a page fetches a rendered form after the initial load and injects it — a single page CRUD, a modal that loads its form, a wizard step — the addModel() call the fragment carries runs when DOMContentLoaded has already been dispatched. The listener is never called, so the form is registered nowhere and keeps no validator at all.

The default of init_js_validation(form) is onLoad = true, so this is the path a fragment hits unless the application knows to pass false. onLoad = false is affected too, in a smaller way: it registers the model right away but still defers disableNativeValidationUi() through the same helper, so with html5_validation: true an injected form keeps the native bubble that stops the submit event this library listens to.

Change

onDocumentReady() runs the callback immediately once document.readyState is past loading, and still waits for the event while the document is being parsed, so the initial page load is unchanged — a model printed inline is still initialized after the document is ready, which is what lets js_validator_config() appear further down the page.

Tests

Two Jest tests in SvarohJsFormValidator.test.js: a model added while the document is ready registers its form, and a model added while document.readyState is loading still waits for the event. The first fails on main.

npx jest — 610 tests pass (608 before). PHP checks are untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

🤖 Generated with Claude Code

Ton Sharpand others added 2 commits August 27, 2026 22:47
"addModel" defers registration to "onDocumentReady", which only added a
"DOMContentLoaded" listener. A form fragment that an application fetches
and injects after the initial load runs its "addModel" call when that
event has already been dispatched, so the listener was never called and
the form kept no validator at all.
The callback now runs immediately once "document.readyState" is past
"loading", and still waits for the event while the document is parsing.
"onDocumentReady" ran the callback for every state past "loading", but
"interactive" means the document is parsed while its deferred scripts,
and the "js_validator_config()" one of them may carry, still run before
"DOMContentLoaded". Only "complete" stands for an event that is really
gone, so the call a model added with "onLoad = false" defers keeps
waiting for the configuration it turns the native UI off with. The same
condition is what old IE needs, where "interactive" does not mean the
document can be walked yet.
The listener the other branch adds is removable again. It passed the
event it received to "removeEventListener", which identifies no
listener, so every call left one behind, and the fallback ran its
callback on every "readystatechange" instead of the last one.
A single page application that swaps a rendered form for a new one left
the element of the node it removed in "formInstances", and every
reopened modal or revisited wizard step added one more. A render that is
no longer in the document is now dropped when the same form is
initialized again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@66Ton99

Copy link
Copy Markdown
Author

Follow-up from review — b7c40c6

A review of the first commit turned up three things the change either introduced or left standing.

interactive is not a document that is done

The immediate branch fired for every state past loading. But interactive means the document is parsed while its deferred scripts still run, and DOMContentLoaded has not been dispatched yet.

That matters because onDocumentReady() does double duty in addModel(). The second call, on the onLoad = false branch, is not a DOM-readiness check at all — it is an ordering barrier, holding disableNativeValidationUi() until a js_validator_config() printed further down the document has set config. A defer or type="module" script would have lost that barrier and left the form without novalidate under html5_validation: true.

The condition is now 'complete' === document.readyState, the only state where the event really is gone. It happens to be what old IE needs too: there interactive does not mean the tree can be walked yet, which is why jQuery pairs its check with documentElement.doScroll.

The listener was never removed

Pre-existing, but the change lives in that function:

addListener.call(document,eventName,function(callee){removeListener.call(this,eventName,callee,false);

callee is the event, not the handler, so removeEventListener() matched nothing and every call left a listener behind. attachEvent passes no argument at all and binds this to window, so the fallback path was broken twice over and ran its callback on every readystatechange instead of the last one. The handler now names itself and detaches from document.

Reopening a modal piled up instances

The scenario the first commit unlocks is a form injected more than once. registerForm() only replaced an instance when domNode was identical, so every re-injection appended one more element pointing at a node that had been taken out of the document, and getFormInstances() handed those out. Instances whose node is no longer in the document are dropped when the same form is initialized again, guarded on document.contains so a browser that cannot answer keeps the old behaviour.

Tests

withReadyState() drives the document state, since jsdom reports complete for a whole run. The existing skips a model without a DOM node on the deferred branch too had quietly stopped deferring — it registered synchronously and its dispatchEvent() was a no-op — so it stubs loading again and asserts it.

Five tests added, each verified to fail on a revert of its own fix and nothing else:

revertedfails
the readyState conditionstill waits for the document while its deferred scripts run, turns the native UI off with a configuration a deferred script sets
the named handlerleaves no listener behind once the document is ready
dropping detached instancesforgets the instance of a render that was taken out of the document

Docs

2_3.md gains two subsections: one on forms loaded after the page — including that innerHTML does not execute a <script> tag it inserts, so a fragment dropped that way is rendered and never initialized — and one holding the existing onLoad = false example. 3_20.md notes the new getFormInstances() behaviour.

Verification

Full suite in the Nix shell, PHP 8.5.6 / Node 24.16.0:

  • composer test — 95 tests, 268 assertions
  • composer phpstan — no errors
  • npm run test:unit — 615 tests
  • npm run test:coverage — 94.67% line coverage, threshold 80%
  • Cypress e2e — 24/24

The e2e run earns its place here: jsdom cannot tell interactive from complete, so a real browser is the only place the ordering claim is exercised end to end.

This supersedes the test count and the "PHP checks were not run" note in the description above.

@66Ton99
66Ton99 merged commit 35440f0 into Svaroh:mainAug 27, 2026
6 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@66Ton99
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(js): initialize a model added after the document is ready - #23

Merged
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:fix/init-model-after-document-ready
Aug 27, 2026
Merged

fix(js): initialize a model added after the document is ready#23
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:fix/init-model-after-document-ready

Conversation

@66Ton99

Copy link
Copy Markdown

Problem

addModel(model, onLoad) defers registration to onDocumentReady(), and that helper only adds a DOMContentLoaded listener:

this.onDocumentReady=function(callback){varaddListener=document.addEventListener||document.attachEvent;
...
addListener.call(document,eventName,function(callee){ ... callback();},false)};

When a page fetches a rendered form after the initial load and injects it — a single page CRUD, a modal that loads its form, a wizard step — the addModel() call the fragment carries runs when DOMContentLoaded has already been dispatched. The listener is never called, so the form is registered nowhere and keeps no validator at all.

The default of init_js_validation(form) is onLoad = true, so this is the path a fragment hits unless the application knows to pass false. onLoad = false is affected too, in a smaller way: it registers the model right away but still defers disableNativeValidationUi() through the same helper, so with html5_validation: true an injected form keeps the native bubble that stops the submit event this library listens to.

Change

onDocumentReady() runs the callback immediately once document.readyState is past loading, and still waits for the event while the document is being parsed, so the initial page load is unchanged — a model printed inline is still initialized after the document is ready, which is what lets js_validator_config() appear further down the page.

Tests

Two Jest tests in SvarohJsFormValidator.test.js: a model added while the document is ready registers its form, and a model added while document.readyState is loading still waits for the event. The first fails on main.

npx jest — 610 tests pass (608 before). PHP checks are untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

🤖 Generated with Claude Code

Ton Sharpand others added 2 commits August 27, 2026 22:47
"addModel" defers registration to "onDocumentReady", which only added a
"DOMContentLoaded" listener. A form fragment that an application fetches
and injects after the initial load runs its "addModel" call when that
event has already been dispatched, so the listener was never called and
the form kept no validator at all.
The callback now runs immediately once "document.readyState" is past
"loading", and still waits for the event while the document is parsing.
"onDocumentReady" ran the callback for every state past "loading", but
"interactive" means the document is parsed while its deferred scripts,
and the "js_validator_config()" one of them may carry, still run before
"DOMContentLoaded". Only "complete" stands for an event that is really
gone, so the call a model added with "onLoad = false" defers keeps
waiting for the configuration it turns the native UI off with. The same
condition is what old IE needs, where "interactive" does not mean the
document can be walked yet.
The listener the other branch adds is removable again. It passed the
event it received to "removeEventListener", which identifies no
listener, so every call left one behind, and the fallback ran its
callback on every "readystatechange" instead of the last one.
A single page application that swaps a rendered form for a new one left
the element of the node it removed in "formInstances", and every
reopened modal or revisited wizard step added one more. A render that is
no longer in the document is now dropped when the same form is
initialized again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@66Ton99

Copy link
Copy Markdown
Author

Follow-up from review — b7c40c6

A review of the first commit turned up three things the change either introduced or left standing.

interactive is not a document that is done

The immediate branch fired for every state past loading. But interactive means the document is parsed while its deferred scripts still run, and DOMContentLoaded has not been dispatched yet.

That matters because onDocumentReady() does double duty in addModel(). The second call, on the onLoad = false branch, is not a DOM-readiness check at all — it is an ordering barrier, holding disableNativeValidationUi() until a js_validator_config() printed further down the document has set config. A defer or type="module" script would have lost that barrier and left the form without novalidate under html5_validation: true.

The condition is now 'complete' === document.readyState, the only state where the event really is gone. It happens to be what old IE needs too: there interactive does not mean the tree can be walked yet, which is why jQuery pairs its check with documentElement.doScroll.

The listener was never removed

Pre-existing, but the change lives in that function:

addListener.call(document,eventName,function(callee){removeListener.call(this,eventName,callee,false);

callee is the event, not the handler, so removeEventListener() matched nothing and every call left a listener behind. attachEvent passes no argument at all and binds this to window, so the fallback path was broken twice over and ran its callback on every readystatechange instead of the last one. The handler now names itself and detaches from document.

Reopening a modal piled up instances

The scenario the first commit unlocks is a form injected more than once. registerForm() only replaced an instance when domNode was identical, so every re-injection appended one more element pointing at a node that had been taken out of the document, and getFormInstances() handed those out. Instances whose node is no longer in the document are dropped when the same form is initialized again, guarded on document.contains so a browser that cannot answer keeps the old behaviour.

Tests

withReadyState() drives the document state, since jsdom reports complete for a whole run. The existing skips a model without a DOM node on the deferred branch too had quietly stopped deferring — it registered synchronously and its dispatchEvent() was a no-op — so it stubs loading again and asserts it.

Five tests added, each verified to fail on a revert of its own fix and nothing else:

revertedfails
the readyState conditionstill waits for the document while its deferred scripts run, turns the native UI off with a configuration a deferred script sets
the named handlerleaves no listener behind once the document is ready
dropping detached instancesforgets the instance of a render that was taken out of the document

Docs

2_3.md gains two subsections: one on forms loaded after the page — including that innerHTML does not execute a <script> tag it inserts, so a fragment dropped that way is rendered and never initialized — and one holding the existing onLoad = false example. 3_20.md notes the new getFormInstances() behaviour.

Verification

Full suite in the Nix shell, PHP 8.5.6 / Node 24.16.0:

  • composer test — 95 tests, 268 assertions
  • composer phpstan — no errors
  • npm run test:unit — 615 tests
  • npm run test:coverage — 94.67% line coverage, threshold 80%
  • Cypress e2e — 24/24

The e2e run earns its place here: jsdom cannot tell interactive from complete, so a real browser is the only place the ordering claim is exercised end to end.

This supersedes the test count and the "PHP checks were not run" note in the description above.

@66Ton99
66Ton99 merged commit 35440f0 into Svaroh:mainAug 27, 2026
6 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@66Ton99
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(js): initialize a model added after the document is ready - #23

Merged
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:fix/init-model-after-document-ready
Aug 27, 2026
Merged

fix(js): initialize a model added after the document is ready#23
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:fix/init-model-after-document-ready

Conversation

@66Ton99

Copy link
Copy Markdown

Problem

addModel(model, onLoad) defers registration to onDocumentReady(), and that helper only adds a DOMContentLoaded listener:

this.onDocumentReady=function(callback){varaddListener=document.addEventListener||document.attachEvent;
...
addListener.call(document,eventName,function(callee){ ... callback();},false)};

When a page fetches a rendered form after the initial load and injects it — a single page CRUD, a modal that loads its form, a wizard step — the addModel() call the fragment carries runs when DOMContentLoaded has already been dispatched. The listener is never called, so the form is registered nowhere and keeps no validator at all.

The default of init_js_validation(form) is onLoad = true, so this is the path a fragment hits unless the application knows to pass false. onLoad = false is affected too, in a smaller way: it registers the model right away but still defers disableNativeValidationUi() through the same helper, so with html5_validation: true an injected form keeps the native bubble that stops the submit event this library listens to.

Change

onDocumentReady() runs the callback immediately once document.readyState is past loading, and still waits for the event while the document is being parsed, so the initial page load is unchanged — a model printed inline is still initialized after the document is ready, which is what lets js_validator_config() appear further down the page.

Tests

Two Jest tests in SvarohJsFormValidator.test.js: a model added while the document is ready registers its form, and a model added while document.readyState is loading still waits for the event. The first fails on main.

npx jest — 610 tests pass (608 before). PHP checks are untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

🤖 Generated with Claude Code

Ton Sharpand others added 2 commits August 27, 2026 22:47
"addModel" defers registration to "onDocumentReady", which only added a
"DOMContentLoaded" listener. A form fragment that an application fetches
and injects after the initial load runs its "addModel" call when that
event has already been dispatched, so the listener was never called and
the form kept no validator at all.
The callback now runs immediately once "document.readyState" is past
"loading", and still waits for the event while the document is parsing.
"onDocumentReady" ran the callback for every state past "loading", but
"interactive" means the document is parsed while its deferred scripts,
and the "js_validator_config()" one of them may carry, still run before
"DOMContentLoaded". Only "complete" stands for an event that is really
gone, so the call a model added with "onLoad = false" defers keeps
waiting for the configuration it turns the native UI off with. The same
condition is what old IE needs, where "interactive" does not mean the
document can be walked yet.
The listener the other branch adds is removable again. It passed the
event it received to "removeEventListener", which identifies no
listener, so every call left one behind, and the fallback ran its
callback on every "readystatechange" instead of the last one.
A single page application that swaps a rendered form for a new one left
the element of the node it removed in "formInstances", and every
reopened modal or revisited wizard step added one more. A render that is
no longer in the document is now dropped when the same form is
initialized again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@66Ton99

Copy link
Copy Markdown
Author

Follow-up from review — b7c40c6

A review of the first commit turned up three things the change either introduced or left standing.

interactive is not a document that is done

The immediate branch fired for every state past loading. But interactive means the document is parsed while its deferred scripts still run, and DOMContentLoaded has not been dispatched yet.

That matters because onDocumentReady() does double duty in addModel(). The second call, on the onLoad = false branch, is not a DOM-readiness check at all — it is an ordering barrier, holding disableNativeValidationUi() until a js_validator_config() printed further down the document has set config. A defer or type="module" script would have lost that barrier and left the form without novalidate under html5_validation: true.

The condition is now 'complete' === document.readyState, the only state where the event really is gone. It happens to be what old IE needs too: there interactive does not mean the tree can be walked yet, which is why jQuery pairs its check with documentElement.doScroll.

The listener was never removed

Pre-existing, but the change lives in that function:

addListener.call(document,eventName,function(callee){removeListener.call(this,eventName,callee,false);

callee is the event, not the handler, so removeEventListener() matched nothing and every call left a listener behind. attachEvent passes no argument at all and binds this to window, so the fallback path was broken twice over and ran its callback on every readystatechange instead of the last one. The handler now names itself and detaches from document.

Reopening a modal piled up instances

The scenario the first commit unlocks is a form injected more than once. registerForm() only replaced an instance when domNode was identical, so every re-injection appended one more element pointing at a node that had been taken out of the document, and getFormInstances() handed those out. Instances whose node is no longer in the document are dropped when the same form is initialized again, guarded on document.contains so a browser that cannot answer keeps the old behaviour.

Tests

withReadyState() drives the document state, since jsdom reports complete for a whole run. The existing skips a model without a DOM node on the deferred branch too had quietly stopped deferring — it registered synchronously and its dispatchEvent() was a no-op — so it stubs loading again and asserts it.

Five tests added, each verified to fail on a revert of its own fix and nothing else:

revertedfails
the readyState conditionstill waits for the document while its deferred scripts run, turns the native UI off with a configuration a deferred script sets
the named handlerleaves no listener behind once the document is ready
dropping detached instancesforgets the instance of a render that was taken out of the document

Docs

2_3.md gains two subsections: one on forms loaded after the page — including that innerHTML does not execute a <script> tag it inserts, so a fragment dropped that way is rendered and never initialized — and one holding the existing onLoad = false example. 3_20.md notes the new getFormInstances() behaviour.

Verification

Full suite in the Nix shell, PHP 8.5.6 / Node 24.16.0:

  • composer test — 95 tests, 268 assertions
  • composer phpstan — no errors
  • npm run test:unit — 615 tests
  • npm run test:coverage — 94.67% line coverage, threshold 80%
  • Cypress e2e — 24/24

The e2e run earns its place here: jsdom cannot tell interactive from complete, so a real browser is the only place the ordering claim is exercised end to end.

This supersedes the test count and the "PHP checks were not run" note in the description above.

@66Ton99
66Ton99 merged commit 35440f0 into Svaroh:mainAug 27, 2026
6 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@66Ton99
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(js): initialize a model added after the document is ready - #23

Merged
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:fix/init-model-after-document-ready
Aug 27, 2026
Merged

fix(js): initialize a model added after the document is ready#23
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:fix/init-model-after-document-ready

Conversation

@66Ton99

Copy link
Copy Markdown

Problem

addModel(model, onLoad) defers registration to onDocumentReady(), and that helper only adds a DOMContentLoaded listener:

this.onDocumentReady=function(callback){varaddListener=document.addEventListener||document.attachEvent;
...
addListener.call(document,eventName,function(callee){ ... callback();},false)};

When a page fetches a rendered form after the initial load and injects it — a single page CRUD, a modal that loads its form, a wizard step — the addModel() call the fragment carries runs when DOMContentLoaded has already been dispatched. The listener is never called, so the form is registered nowhere and keeps no validator at all.

The default of init_js_validation(form) is onLoad = true, so this is the path a fragment hits unless the application knows to pass false. onLoad = false is affected too, in a smaller way: it registers the model right away but still defers disableNativeValidationUi() through the same helper, so with html5_validation: true an injected form keeps the native bubble that stops the submit event this library listens to.

Change

onDocumentReady() runs the callback immediately once document.readyState is past loading, and still waits for the event while the document is being parsed, so the initial page load is unchanged — a model printed inline is still initialized after the document is ready, which is what lets js_validator_config() appear further down the page.

Tests

Two Jest tests in SvarohJsFormValidator.test.js: a model added while the document is ready registers its form, and a model added while document.readyState is loading still waits for the event. The first fails on main.

npx jest — 610 tests pass (608 before). PHP checks are untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

🤖 Generated with Claude Code

Ton Sharpand others added 2 commits August 27, 2026 22:47
"addModel" defers registration to "onDocumentReady", which only added a
"DOMContentLoaded" listener. A form fragment that an application fetches
and injects after the initial load runs its "addModel" call when that
event has already been dispatched, so the listener was never called and
the form kept no validator at all.
The callback now runs immediately once "document.readyState" is past
"loading", and still waits for the event while the document is parsing.
"onDocumentReady" ran the callback for every state past "loading", but
"interactive" means the document is parsed while its deferred scripts,
and the "js_validator_config()" one of them may carry, still run before
"DOMContentLoaded". Only "complete" stands for an event that is really
gone, so the call a model added with "onLoad = false" defers keeps
waiting for the configuration it turns the native UI off with. The same
condition is what old IE needs, where "interactive" does not mean the
document can be walked yet.
The listener the other branch adds is removable again. It passed the
event it received to "removeEventListener", which identifies no
listener, so every call left one behind, and the fallback ran its
callback on every "readystatechange" instead of the last one.
A single page application that swaps a rendered form for a new one left
the element of the node it removed in "formInstances", and every
reopened modal or revisited wizard step added one more. A render that is
no longer in the document is now dropped when the same form is
initialized again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@66Ton99

Copy link
Copy Markdown
Author

Follow-up from review — b7c40c6

A review of the first commit turned up three things the change either introduced or left standing.

interactive is not a document that is done

The immediate branch fired for every state past loading. But interactive means the document is parsed while its deferred scripts still run, and DOMContentLoaded has not been dispatched yet.

That matters because onDocumentReady() does double duty in addModel(). The second call, on the onLoad = false branch, is not a DOM-readiness check at all — it is an ordering barrier, holding disableNativeValidationUi() until a js_validator_config() printed further down the document has set config. A defer or type="module" script would have lost that barrier and left the form without novalidate under html5_validation: true.

The condition is now 'complete' === document.readyState, the only state where the event really is gone. It happens to be what old IE needs too: there interactive does not mean the tree can be walked yet, which is why jQuery pairs its check with documentElement.doScroll.

The listener was never removed

Pre-existing, but the change lives in that function:

addListener.call(document,eventName,function(callee){removeListener.call(this,eventName,callee,false);

callee is the event, not the handler, so removeEventListener() matched nothing and every call left a listener behind. attachEvent passes no argument at all and binds this to window, so the fallback path was broken twice over and ran its callback on every readystatechange instead of the last one. The handler now names itself and detaches from document.

Reopening a modal piled up instances

The scenario the first commit unlocks is a form injected more than once. registerForm() only replaced an instance when domNode was identical, so every re-injection appended one more element pointing at a node that had been taken out of the document, and getFormInstances() handed those out. Instances whose node is no longer in the document are dropped when the same form is initialized again, guarded on document.contains so a browser that cannot answer keeps the old behaviour.

Tests

withReadyState() drives the document state, since jsdom reports complete for a whole run. The existing skips a model without a DOM node on the deferred branch too had quietly stopped deferring — it registered synchronously and its dispatchEvent() was a no-op — so it stubs loading again and asserts it.

Five tests added, each verified to fail on a revert of its own fix and nothing else:

revertedfails
the readyState conditionstill waits for the document while its deferred scripts run, turns the native UI off with a configuration a deferred script sets
the named handlerleaves no listener behind once the document is ready
dropping detached instancesforgets the instance of a render that was taken out of the document

Docs

2_3.md gains two subsections: one on forms loaded after the page — including that innerHTML does not execute a <script> tag it inserts, so a fragment dropped that way is rendered and never initialized — and one holding the existing onLoad = false example. 3_20.md notes the new getFormInstances() behaviour.

Verification

Full suite in the Nix shell, PHP 8.5.6 / Node 24.16.0:

  • composer test — 95 tests, 268 assertions
  • composer phpstan — no errors
  • npm run test:unit — 615 tests
  • npm run test:coverage — 94.67% line coverage, threshold 80%
  • Cypress e2e — 24/24

The e2e run earns its place here: jsdom cannot tell interactive from complete, so a real browser is the only place the ordering claim is exercised end to end.

This supersedes the test count and the "PHP checks were not run" note in the description above.

@66Ton99
66Ton99 merged commit 35440f0 into Svaroh:mainAug 27, 2026
6 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@66Ton99
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(js): initialize a model added after the document is ready - #23

Merged
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:fix/init-model-after-document-ready
Aug 27, 2026
Merged

fix(js): initialize a model added after the document is ready#23
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:fix/init-model-after-document-ready

Conversation

@66Ton99

Copy link
Copy Markdown

Problem

addModel(model, onLoad) defers registration to onDocumentReady(), and that helper only adds a DOMContentLoaded listener:

this.onDocumentReady=function(callback){varaddListener=document.addEventListener||document.attachEvent;
...
addListener.call(document,eventName,function(callee){ ... callback();},false)};

When a page fetches a rendered form after the initial load and injects it — a single page CRUD, a modal that loads its form, a wizard step — the addModel() call the fragment carries runs when DOMContentLoaded has already been dispatched. The listener is never called, so the form is registered nowhere and keeps no validator at all.

The default of init_js_validation(form) is onLoad = true, so this is the path a fragment hits unless the application knows to pass false. onLoad = false is affected too, in a smaller way: it registers the model right away but still defers disableNativeValidationUi() through the same helper, so with html5_validation: true an injected form keeps the native bubble that stops the submit event this library listens to.

Change

onDocumentReady() runs the callback immediately once document.readyState is past loading, and still waits for the event while the document is being parsed, so the initial page load is unchanged — a model printed inline is still initialized after the document is ready, which is what lets js_validator_config() appear further down the page.

Tests

Two Jest tests in SvarohJsFormValidator.test.js: a model added while the document is ready registers its form, and a model added while document.readyState is loading still waits for the event. The first fails on main.

npx jest — 610 tests pass (608 before). PHP checks are untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

🤖 Generated with Claude Code

Ton Sharpand others added 2 commits August 27, 2026 22:47
"addModel" defers registration to "onDocumentReady", which only added a
"DOMContentLoaded" listener. A form fragment that an application fetches
and injects after the initial load runs its "addModel" call when that
event has already been dispatched, so the listener was never called and
the form kept no validator at all.
The callback now runs immediately once "document.readyState" is past
"loading", and still waits for the event while the document is parsing.
"onDocumentReady" ran the callback for every state past "loading", but
"interactive" means the document is parsed while its deferred scripts,
and the "js_validator_config()" one of them may carry, still run before
"DOMContentLoaded". Only "complete" stands for an event that is really
gone, so the call a model added with "onLoad = false" defers keeps
waiting for the configuration it turns the native UI off with. The same
condition is what old IE needs, where "interactive" does not mean the
document can be walked yet.
The listener the other branch adds is removable again. It passed the
event it received to "removeEventListener", which identifies no
listener, so every call left one behind, and the fallback ran its
callback on every "readystatechange" instead of the last one.
A single page application that swaps a rendered form for a new one left
the element of the node it removed in "formInstances", and every
reopened modal or revisited wizard step added one more. A render that is
no longer in the document is now dropped when the same form is
initialized again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@66Ton99

Copy link
Copy Markdown
Author

Follow-up from review — b7c40c6

A review of the first commit turned up three things the change either introduced or left standing.

interactive is not a document that is done

The immediate branch fired for every state past loading. But interactive means the document is parsed while its deferred scripts still run, and DOMContentLoaded has not been dispatched yet.

That matters because onDocumentReady() does double duty in addModel(). The second call, on the onLoad = false branch, is not a DOM-readiness check at all — it is an ordering barrier, holding disableNativeValidationUi() until a js_validator_config() printed further down the document has set config. A defer or type="module" script would have lost that barrier and left the form without novalidate under html5_validation: true.

The condition is now 'complete' === document.readyState, the only state where the event really is gone. It happens to be what old IE needs too: there interactive does not mean the tree can be walked yet, which is why jQuery pairs its check with documentElement.doScroll.

The listener was never removed

Pre-existing, but the change lives in that function:

addListener.call(document,eventName,function(callee){removeListener.call(this,eventName,callee,false);

callee is the event, not the handler, so removeEventListener() matched nothing and every call left a listener behind. attachEvent passes no argument at all and binds this to window, so the fallback path was broken twice over and ran its callback on every readystatechange instead of the last one. The handler now names itself and detaches from document.

Reopening a modal piled up instances

The scenario the first commit unlocks is a form injected more than once. registerForm() only replaced an instance when domNode was identical, so every re-injection appended one more element pointing at a node that had been taken out of the document, and getFormInstances() handed those out. Instances whose node is no longer in the document are dropped when the same form is initialized again, guarded on document.contains so a browser that cannot answer keeps the old behaviour.

Tests

withReadyState() drives the document state, since jsdom reports complete for a whole run. The existing skips a model without a DOM node on the deferred branch too had quietly stopped deferring — it registered synchronously and its dispatchEvent() was a no-op — so it stubs loading again and asserts it.

Five tests added, each verified to fail on a revert of its own fix and nothing else:

revertedfails
the readyState conditionstill waits for the document while its deferred scripts run, turns the native UI off with a configuration a deferred script sets
the named handlerleaves no listener behind once the document is ready
dropping detached instancesforgets the instance of a render that was taken out of the document

Docs

2_3.md gains two subsections: one on forms loaded after the page — including that innerHTML does not execute a <script> tag it inserts, so a fragment dropped that way is rendered and never initialized — and one holding the existing onLoad = false example. 3_20.md notes the new getFormInstances() behaviour.

Verification

Full suite in the Nix shell, PHP 8.5.6 / Node 24.16.0:

  • composer test — 95 tests, 268 assertions
  • composer phpstan — no errors
  • npm run test:unit — 615 tests
  • npm run test:coverage — 94.67% line coverage, threshold 80%
  • Cypress e2e — 24/24

The e2e run earns its place here: jsdom cannot tell interactive from complete, so a real browser is the only place the ordering claim is exercised end to end.

This supersedes the test count and the "PHP checks were not run" note in the description above.

@66Ton99
66Ton99 merged commit 35440f0 into Svaroh:mainAug 27, 2026
6 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@66Ton99
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(js): initialize a model added after the document is ready - #23

Merged
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:fix/init-model-after-document-ready
Aug 27, 2026
Merged

fix(js): initialize a model added after the document is ready#23
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:fix/init-model-after-document-ready

Conversation

@66Ton99

Copy link
Copy Markdown

Problem

addModel(model, onLoad) defers registration to onDocumentReady(), and that helper only adds a DOMContentLoaded listener:

this.onDocumentReady=function(callback){varaddListener=document.addEventListener||document.attachEvent;
...
addListener.call(document,eventName,function(callee){ ... callback();},false)};

When a page fetches a rendered form after the initial load and injects it — a single page CRUD, a modal that loads its form, a wizard step — the addModel() call the fragment carries runs when DOMContentLoaded has already been dispatched. The listener is never called, so the form is registered nowhere and keeps no validator at all.

The default of init_js_validation(form) is onLoad = true, so this is the path a fragment hits unless the application knows to pass false. onLoad = false is affected too, in a smaller way: it registers the model right away but still defers disableNativeValidationUi() through the same helper, so with html5_validation: true an injected form keeps the native bubble that stops the submit event this library listens to.

Change

onDocumentReady() runs the callback immediately once document.readyState is past loading, and still waits for the event while the document is being parsed, so the initial page load is unchanged — a model printed inline is still initialized after the document is ready, which is what lets js_validator_config() appear further down the page.

Tests

Two Jest tests in SvarohJsFormValidator.test.js: a model added while the document is ready registers its form, and a model added while document.readyState is loading still waits for the event. The first fails on main.

npx jest — 610 tests pass (608 before). PHP checks are untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

🤖 Generated with Claude Code

Ton Sharpand others added 2 commits August 27, 2026 22:47
"addModel" defers registration to "onDocumentReady", which only added a
"DOMContentLoaded" listener. A form fragment that an application fetches
and injects after the initial load runs its "addModel" call when that
event has already been dispatched, so the listener was never called and
the form kept no validator at all.
The callback now runs immediately once "document.readyState" is past
"loading", and still waits for the event while the document is parsing.
"onDocumentReady" ran the callback for every state past "loading", but
"interactive" means the document is parsed while its deferred scripts,
and the "js_validator_config()" one of them may carry, still run before
"DOMContentLoaded". Only "complete" stands for an event that is really
gone, so the call a model added with "onLoad = false" defers keeps
waiting for the configuration it turns the native UI off with. The same
condition is what old IE needs, where "interactive" does not mean the
document can be walked yet.
The listener the other branch adds is removable again. It passed the
event it received to "removeEventListener", which identifies no
listener, so every call left one behind, and the fallback ran its
callback on every "readystatechange" instead of the last one.
A single page application that swaps a rendered form for a new one left
the element of the node it removed in "formInstances", and every
reopened modal or revisited wizard step added one more. A render that is
no longer in the document is now dropped when the same form is
initialized again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@66Ton99

Copy link
Copy Markdown
Author

Follow-up from review — b7c40c6

A review of the first commit turned up three things the change either introduced or left standing.

interactive is not a document that is done

The immediate branch fired for every state past loading. But interactive means the document is parsed while its deferred scripts still run, and DOMContentLoaded has not been dispatched yet.

That matters because onDocumentReady() does double duty in addModel(). The second call, on the onLoad = false branch, is not a DOM-readiness check at all — it is an ordering barrier, holding disableNativeValidationUi() until a js_validator_config() printed further down the document has set config. A defer or type="module" script would have lost that barrier and left the form without novalidate under html5_validation: true.

The condition is now 'complete' === document.readyState, the only state where the event really is gone. It happens to be what old IE needs too: there interactive does not mean the tree can be walked yet, which is why jQuery pairs its check with documentElement.doScroll.

The listener was never removed

Pre-existing, but the change lives in that function:

addListener.call(document,eventName,function(callee){removeListener.call(this,eventName,callee,false);

callee is the event, not the handler, so removeEventListener() matched nothing and every call left a listener behind. attachEvent passes no argument at all and binds this to window, so the fallback path was broken twice over and ran its callback on every readystatechange instead of the last one. The handler now names itself and detaches from document.

Reopening a modal piled up instances

The scenario the first commit unlocks is a form injected more than once. registerForm() only replaced an instance when domNode was identical, so every re-injection appended one more element pointing at a node that had been taken out of the document, and getFormInstances() handed those out. Instances whose node is no longer in the document are dropped when the same form is initialized again, guarded on document.contains so a browser that cannot answer keeps the old behaviour.

Tests

withReadyState() drives the document state, since jsdom reports complete for a whole run. The existing skips a model without a DOM node on the deferred branch too had quietly stopped deferring — it registered synchronously and its dispatchEvent() was a no-op — so it stubs loading again and asserts it.

Five tests added, each verified to fail on a revert of its own fix and nothing else:

revertedfails
the readyState conditionstill waits for the document while its deferred scripts run, turns the native UI off with a configuration a deferred script sets
the named handlerleaves no listener behind once the document is ready
dropping detached instancesforgets the instance of a render that was taken out of the document

Docs

2_3.md gains two subsections: one on forms loaded after the page — including that innerHTML does not execute a <script> tag it inserts, so a fragment dropped that way is rendered and never initialized — and one holding the existing onLoad = false example. 3_20.md notes the new getFormInstances() behaviour.

Verification

Full suite in the Nix shell, PHP 8.5.6 / Node 24.16.0:

  • composer test — 95 tests, 268 assertions
  • composer phpstan — no errors
  • npm run test:unit — 615 tests
  • npm run test:coverage — 94.67% line coverage, threshold 80%
  • Cypress e2e — 24/24

The e2e run earns its place here: jsdom cannot tell interactive from complete, so a real browser is the only place the ordering claim is exercised end to end.

This supersedes the test count and the "PHP checks were not run" note in the description above.

@66Ton99
66Ton99 merged commit 35440f0 into Svaroh:mainAug 27, 2026
6 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@66Ton99