Fix: Append accent color only to last <style>, avoid loss in Admin preview #24722
Conversation
WalkthroughIn ghost/core/core/frontend/helpers/ghost_head.js, the logic for placing the accent color style tag was adjusted. The search for an insertion point now targets only the last closing </style> tag using /</style>/, replacing the previous check for either </style> or </script> via /</(style|script)>/. The variable was renamed from existingScriptIndex to existingStyleIndex. If a closing </style> is found (existingStyleIndex !== -1), the style is appended to it; otherwise, a new <style>:root {--ghost-accent-color: ...}</style> tag is added. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
ghost/core/core/frontend/helpers/ghost_head.js (2)
340-343: Make the matcher resilient (case-insensitive, optional whitespace) and stringify nodes.Some entries in
headare SafeString objects and user/3rd-party injections may close</STYLE>with different casing or whitespace. Minor hardening below:Apply this diff:
- const existingStyleIndex = _.findLastIndex(head, str => /<\/style>/.test(str)); + // Be robust to SafeString objects, casing, and stray whitespace before '>' + const existingStyleIndex = _.findLastIndex( + head, + (node) => /<\/style\s*>/i.test(String(node)) + );
342-343: Add a newline when concatenating for readability (nit).Purely cosmetic; keeps the generated markup readable when multiple styles are concatenated into a single array entry.
- head[existingStyleIndex] = head[existingStyleIndex] + styleTag; + head[existingStyleIndex] = `${head[existingStyleIndex]}\n ${styleTag}`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ghost/core/core/frontend/helpers/ghost_head.js(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kevinansfield
PR: TryGhost/Ghost#23824
File: ghost/core/core/server/services/email-service/email-templates/partials/styles.hbs:919-926
Timestamp: 2025-06-13T11:57:58.226Z
Learning: In `ghost/core/core/server/services/email-service/email-templates/partials/styles.hbs`, some style blocks (e.g., `.latest-post p` and `.latest-post p a`) still use the legacy colour `#73818c` on purpose; they are later overridden by `emailCustomization` feature rules, as noted by inline TODO comments. These occurrences should not be flagged as inconsistencies.
📚 Learning: 2025-06-13T11:57:58.226Z
Learnt from: kevinansfield
PR: TryGhost/Ghost#23824
File: ghost/core/core/server/services/email-service/email-templates/partials/styles.hbs:919-926
Timestamp: 2025-06-13T11:57:58.226Z
Learning: In `ghost/core/core/server/services/email-service/email-templates/partials/styles.hbs`, some style blocks (e.g., `.latest-post p` and `.latest-post p a`) still use the legacy colour `#73818c` on purpose; they are later overridden by `emailCustomization` feature rules, as noted by inline TODO comments. These occurrences should not be flagged as inconsistencies.
Applied to files:
ghost/core/core/frontend/helpers/ghost_head.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Ghost-CLI tests
- GitHub Check: E2E tests
- GitHub Check: Acceptance tests (Node 22.13.1, sqlite3)
- GitHub Check: Acceptance tests (Node 22.13.1, mysql8)
- GitHub Check: Legacy tests (Node 22.13.1, sqlite3)
- GitHub Check: Unit tests (Node 22.13.1)
- GitHub Check: Legacy tests (Node 22.13.1, mysql8)
- GitHub Check: Lint
🔇 Additional comments (1)
ghost/core/core/frontend/helpers/ghost_head.js (1)
340-343: Good fix: limiting the insertion point to only the last </style> avoids Admin Preview losses.Targeting only style blocks decouples the accent style from script sanitization/removal in Admin Preview and matches the intent described in the PR. Nice, minimal surface-area change.
When using the
{{ghost_head}}helper with the exclude option (for example excluding{{ghost_head exclude="comment_counts"}}), the accent color style can disappear in the Admin Preview.The issue occurs because the accent style is currently appended to the last style or script element in the head output. In the Admin Preview environment, script tags are sanitized or removed, and if the accent style was attached to one of them, it gets lost. This leads to the accent color not appearing in Admin, while still working correctly on the frontend. The inconsistency is confusing for theme developers and customers testing themes inside Admin.
Fix:
The injection logic has been adjusted so that the accent color is only appended to the last style element in the head output. If no style element exists, it will be added as a separate entry. This ensures that the accent color is never tied to a script element that could be removed in Admin Preview.