[PR #4688] [MERGED] New include tag parser #6395

New Issue

OVERLORD · 2026-02-05T10:31:10+03:00

OVERLORD commented

2026-02-05 10:31:10 +03:00

📋 Pull Request Information

Original PR: https://github.com/BookStackApp/BookStack/pull/4688
Author: @ssddanbrown
Created: 11/23/2023
Status: ✅ Merged
Merged: 11/28/2023
Merged by: @ssddanbrown

Base: development ← Head: include-parser

📝 Commits (7)

04d21c8 Includes: Started foundations for new include tag parser
7593645 Includes: Developed to get new system working with inline includes
c88eb72 Includes: Added block-level handling to new include system
4874dc1 Includes: Updated logic regarding parent block els, added tests
71c93c8 Includes: Switched page to new system
b569827 Includes: Added ID de-duplicating and more thorough clean-up
652d541 Includes: Added back support for parse theme event

📊 Changes

10 files changed (+669 additions, -134 deletions)

View changed files

📝 app/Entities/Tools/PageContent.php (+54 -88)
➕ app/Entities/Tools/PageIncludeContent.php (+85 -0)
➕ app/Entities/Tools/PageIncludeParser.php (+220 -0)
➕ app/Entities/Tools/PageIncludeTag.php (+30 -0)
📝 app/Theming/CustomHtmlHeadContentProvider.php (+1 -1)
📝 app/Theming/ThemeEvents.php (+2 -4)
📝 app/Theming/ThemeService.php (+8 -0)
📝 app/Util/HtmlContentFilter.php (+15 -8)
📝 tests/Entity/PageContentTest.php (+14 -33)
➕ tests/Unit/PageIncludeParserTest.php (+240 -0)

📄 Description

Todo

Consider usage with details blocks (Maybe other acceptable top levels? Could be worth inverting to handle blocks that don't allow this.
- This page has a list: html, head, body, p, dt, dd, li, ption, thead, th, tbody, tr, td, tfoot, colgroup.
- In the end, I made this specific to p tags since that's the large case to target, otherwise most existing parents should be fine. Will be edge-cases (header in other headers etc...) but are much more likely to be user-driven and solvable, and harder to fall into.
Add multi-depth support (3 levels as per production)
Look to update/handle the logical theme system event somehow.
Handle duplicate IDs?
Expand test cases
- Multiple tags in same elem.

Doc Updates

Note (potentially) breaking change of include behaviour as update advisory.
~~Note break/change in include event~~.
Maybe update page include docs with some technical info regarding it's behaviour?

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/BookStackApp/BookStack/pull/4688 **Author:** [@ssddanbrown](https://github.com/ssddanbrown) **Created:** 11/23/2023 **Status:** ✅ Merged **Merged:** 11/28/2023 **Merged by:** [@ssddanbrown](https://github.com/ssddanbrown) **Base:** `development` ← **Head:** `include-parser` --- ### 📝 Commits (7) - [`04d21c8`](https://github.com/BookStackApp/BookStack/commit/04d21c8a97da9463e6bedda620ecf1282722ea3e) Includes: Started foundations for new include tag parser - [`7593645`](https://github.com/BookStackApp/BookStack/commit/75936454cca139d0b226a95ee7a0070bc8702fdc) Includes: Developed to get new system working with inline includes - [`c88eb72`](https://github.com/BookStackApp/BookStack/commit/c88eb729a499197e8b3ab9d5019b4426a65d3d41) Includes: Added block-level handling to new include system - [`4874dc1`](https://github.com/BookStackApp/BookStack/commit/4874dc1304ee26737884d787893743ae323e5cf2) Includes: Updated logic regarding parent block els, added tests - [`71c93c8`](https://github.com/BookStackApp/BookStack/commit/71c93c88787fe3522d1f0f49d445eb24e3a2789e) Includes: Switched page to new system - [`b569827`](https://github.com/BookStackApp/BookStack/commit/b569827114693aec0f143a54dce5ddc693174734) Includes: Added ID de-duplicating and more thorough clean-up - [`652d541`](https://github.com/BookStackApp/BookStack/commit/652d5417bf5dbd9700412ad82fdc1a783c83e5a8) Includes: Added back support for parse theme event ### 📊 Changes **10 files changed** (+669 additions, -134 deletions) <details> <summary>View changed files</summary> 📝 `app/Entities/Tools/PageContent.php` (+54 -88) ➕ `app/Entities/Tools/PageIncludeContent.php` (+85 -0) ➕ `app/Entities/Tools/PageIncludeParser.php` (+220 -0) ➕ `app/Entities/Tools/PageIncludeTag.php` (+30 -0) 📝 `app/Theming/CustomHtmlHeadContentProvider.php` (+1 -1) 📝 `app/Theming/ThemeEvents.php` (+2 -4) 📝 `app/Theming/ThemeService.php` (+8 -0) 📝 `app/Util/HtmlContentFilter.php` (+15 -8) 📝 `tests/Entity/PageContentTest.php` (+14 -33) ➕ `tests/Unit/PageIncludeParserTest.php` (+240 -0) </details> ### 📄 Description ### Todo - [x] Consider usage with details blocks (Maybe other acceptable top levels? Could be worth inverting to handle blocks that don't allow this. - [This page has a list](https://blog.teamtreehouse.com/to-close-or-not-to-close-tags-in-html5): html, head, body, p, dt, dd, li, ption, thead, th, tbody, tr, td, tfoot, colgroup. - In the end, I made this specific to `p` tags since that's the large case to target, otherwise most existing parents should be fine. Will be edge-cases (header in other headers etc...) but are much more likely to be user-driven and solvable, and harder to fall into. - [x] Add multi-depth support (3 levels as per production) - [x] Look to update/handle the logical theme system event somehow. - [x] Handle duplicate IDs? - [x] Expand test cases - [x] Multiple tags in same elem. ### Doc Updates - Note (potentially) breaking change of include behaviour as update advisory. - ~~Note break/change in include event~~. - Maybe update page include docs with some technical info regarding it's behaviour? --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

OVERLORD added the pull-request label 2026-02-05 10:31:10 +03:00

OVERLORD closed this issue

2026-02-05 10:31:13 +03:00

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/BookStack#6395