Skip to content

fix: inject CSS/script into body when head tag is missing#6

Merged
jergason merged 1 commit intomainfrom
fix/inject-into-body-fallback
Mar 23, 2026
Merged

fix: inject CSS/script into body when head tag is missing#6
jergason merged 1 commit intomainfrom
fix/inject-into-body-fallback

Conversation

@jergason
Copy link
Copy Markdown
Owner

Summary

  • Pages like old.reddit.com have no <head> tag — HTML starts directly with <body>. Our HeadInjector only fired on <head>, so these pages got zero proxy functionality: no CSS text-transform, no uppercase script, no fetch/XHR patching.
  • Replaces HeadInjector with ScriptAndStyleInjector that registers on both <head> and <body>
  • Uses injected flag to prevent double injection on pages that have both tags
  • Appends to <head> (normal case), prepends to <body> (fallback)

Test plan

  • 34 tests pass (including new no-double-injection test)
  • All checks clean: lint, format, types, knip
  • Verified old.reddit.com now gets CSS + script injected
  • Verified HN (normal page with <head>) has exactly 1 CSS injection, no duplication
  • Batch tested 16 sites — no regressions

🤖 Generated with Claude Code

pages like old.reddit.com start with <body> and have no <head> tag.
our HeadInjector only fired on <head>, so these pages got no CSS
text-transform, no uppercase script, no fetch/XHR patching — nothing.

replaces HeadInjector with ScriptAndStyleInjector that registers on
both head and body. uses an injected flag to prevent double injection.
appends to head (normal), prepends to body (fallback).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 23, 2026 17:39
@jergason jergason merged commit 75ade3f into main Mar 23, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes missing proxy behavior on HTML pages that omit a <head> element by injecting the proxy CSS/script into either <head> (normal) or <body> (fallback), while preventing double-injection when both tags exist.

Changes:

  • Replace HeadInjector with a ScriptAndStyleInjector that runs on both head and body.
  • Add an injected guard to ensure CSS/script are injected only once per document.
  • Add a regression test intended to detect double-injection on pages that have both <head> and <body>.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/rewriter.ts Adds shared injector registered on both head and body with a single-injection guard.
src/rewriter.test.ts Adds a test attempting to ensure CSS/script aren’t injected twice.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/rewriter.ts
Comment on lines 89 to +95
element(el: Element) {
// no <base> tag — it would redirect /browse/... paths to the target origin
// URLRewriter already resolves all relative URLs to absolute proxy paths
el.append(
`<style>*:not(input):not(textarea):not(select):not(code):not(pre):not(script):not(style) { text-transform: uppercase !important; } code, pre, textarea, svg { text-transform: none !important; }</style>`,
{ html: true },
);
el.append(`<script>${uppercaseScript}</script>`, { html: true });
if (this.injected) return;
this.injected = true;
// prepend into body (fallback for pages without <head>), append into head
const method = el.tagName === "head" ? "append" : "prepend";
el[method](INJECTED_CSS, { html: true });
el[method](`<script>${uppercaseScript}</script>`, { html: true });
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new body-fallback injection path (pages without a ) isn’t covered by an automated test. Consider adding a deterministic integration/unit test that transforms a minimal HTML document starting with (no ) and asserts the CSS/script are injected (and placed in/at the start of body).

Copilot uses AI. Check for mistakes.
Comment thread src/rewriter.test.ts
Comment on lines +117 to +121
const scriptCount = (html.match(/walkAndUppercase/g) || []).length;
// walkAndUppercase appears multiple times within the single script (definition + calls)
// but should NOT appear in a second duplicate script block
expect(scriptCount).toBeGreaterThan(0);
expect(scriptCount).toBeLessThan(10);
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test’s duplicate-script assertion is brittle: counting raw "walkAndUppercase" occurrences and bounding it (<10) will break if the script implementation changes (e.g., additional references). A more stable check is to assert a unique signature like "function walkAndUppercase" (or another sentinel) appears exactly once, or to count injected <script> blocks matching the injector output.

Suggested change
const scriptCount = (html.match(/walkAndUppercase/g) || []).length;
// walkAndUppercase appears multiple times within the single script (definition + calls)
// but should NOT appear in a second duplicate script block
expect(scriptCount).toBeGreaterThan(0);
expect(scriptCount).toBeLessThan(10);
const scriptDefCount = (html.match(/function walkAndUppercase\s*\(/g) || []).length;
// Ensure the walkAndUppercase script is injected exactly once (no duplicate script blocks)
expect(scriptDefCount).toBe(1);

Copilot uses AI. Check for mistakes.
Sign up for free to 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.

2 participants