From 652d5417bf5dbd9700412ad82fdc1a783c83e5a8 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 27 Nov 2023 21:38:43 +0000 Subject: [PATCH] Includes: Added back support for parse theme event Managed to do this in an API-compatible way although resuling output may differ due to new dom handling in general, although user content is used inline to remain as comptable as possible. --- app/Entities/Tools/PageContent.php | 49 +++++++++++++++++----- app/Entities/Tools/PageIncludeContent.php | 51 +++++++++++++++-------- app/Entities/Tools/PageIncludeParser.php | 7 +++- app/Theming/ThemeEvents.php | 6 +-- app/Theming/ThemeService.php | 8 ++++ tests/Entity/PageContentTest.php | 13 ++++++ tests/Unit/PageIncludeParserTest.php | 7 +++- 7 files changed, 106 insertions(+), 35 deletions(-) diff --git a/app/Entities/Tools/PageContent.php b/app/Entities/Tools/PageContent.php index 22190f03f..99070ae89 100644 --- a/app/Entities/Tools/PageContent.php +++ b/app/Entities/Tools/PageContent.php @@ -5,10 +5,13 @@ namespace BookStack\Entities\Tools; use BookStack\Entities\Models\Page; use BookStack\Entities\Tools\Markdown\MarkdownToHtml; use BookStack\Exceptions\ImageUploadException; +use BookStack\Facades\Theme; +use BookStack\Theming\ThemeEvents; use BookStack\Uploads\ImageRepo; use BookStack\Uploads\ImageService; use BookStack\Util\HtmlContentFilter; use BookStack\Util\HtmlDocument; +use Closure; use DOMElement; use DOMNode; use DOMNodeList; @@ -280,18 +283,11 @@ class PageContent } $doc = new HtmlDocument($html); - - $contentProvider = function (int $id) use ($blankIncludes) { - if ($blankIncludes) { - return ''; - } - return Page::visible()->find($id)->html ?? ''; - }; - + $contentProvider = $this->getContentProviderClosure($blankIncludes); $parser = new PageIncludeParser($doc, $contentProvider); - $nodesAdded = 1; - for ($includeDepth = 0; $includeDepth < 1 && $nodesAdded !== 0; $includeDepth++) { + $nodesAdded = 1; + for ($includeDepth = 0; $includeDepth < 3 && $nodesAdded !== 0; $includeDepth++) { $nodesAdded = $parser->parse(); } @@ -308,6 +304,39 @@ class PageContent return $doc->getBodyInnerHtml(); } + /** + * Get the closure used to fetch content for page includes. + */ + protected function getContentProviderClosure(bool $blankIncludes): Closure + { + $contextPage = $this->page; + + return function (PageIncludeTag $tag) use ($blankIncludes, $contextPage): PageIncludeContent { + if ($blankIncludes) { + return PageIncludeContent::fromHtmlAndTag('', $tag); + } + + $matchedPage = Page::visible()->find($tag->getPageId()); + $content = PageIncludeContent::fromHtmlAndTag($matchedPage->html ?? '', $tag); + + if (Theme::hasListeners(ThemeEvents::PAGE_INCLUDE_PARSE)) { + $themeReplacement = Theme::dispatch( + ThemeEvents::PAGE_INCLUDE_PARSE, + $tag->tagContent, + $content->toHtml(), + clone $contextPage, + $matchedPage ? (clone $matchedPage) : null, + ); + + if ($themeReplacement !== null) { + $content = PageIncludeContent::fromInlineHtml(strval($themeReplacement)); + } + } + + return $content; + }; + } + /** * Parse the headers on the page to get a navigation menu. */ diff --git a/app/Entities/Tools/PageIncludeContent.php b/app/Entities/Tools/PageIncludeContent.php index 2e173859d..7c4f943c8 100644 --- a/app/Entities/Tools/PageIncludeContent.php +++ b/app/Entities/Tools/PageIncludeContent.php @@ -10,47 +10,53 @@ class PageIncludeContent protected static array $topLevelTags = ['table', 'ul', 'ol', 'pre']; /** - * @var DOMNode[] + * @param DOMNode[] $contents + * @param bool $isInline */ - protected array $contents = []; - - protected bool $isTopLevel = false; - public function __construct( - string $html, - PageIncludeTag $tag, + protected array $contents, + protected bool $isInline, ) { - $this->parseHtml($html, $tag); } - protected function parseHtml(string $html, PageIncludeTag $tag): void + public static function fromHtmlAndTag(string $html, PageIncludeTag $tag): self { if (empty($html)) { - return; + return new self([], true); } $doc = new HtmlDocument($html); $sectionId = $tag->getSectionId(); if (!$sectionId) { - $this->contents = [...$doc->getBodyChildren()]; - $this->isTopLevel = true; - return; + $contents = [...$doc->getBodyChildren()]; + return new self($contents, false); } $section = $doc->getElementById($sectionId); if (!$section) { - return; + return new self([], true); } $isTopLevel = in_array(strtolower($section->nodeName), static::$topLevelTags); - $this->isTopLevel = $isTopLevel; - $this->contents = $isTopLevel ? [$section] : [...$section->childNodes]; + $contents = $isTopLevel ? [$section] : [...$section->childNodes]; + return new self($contents, !$isTopLevel); + } + + public static function fromInlineHtml(string $html): self + { + if (empty($html)) { + return new self([], true); + } + + $doc = new HtmlDocument($html); + + return new self([...$doc->getBodyChildren()], true); } public function isInline(): bool { - return !$this->isTopLevel; + return $this->isInline; } public function isEmpty(): bool @@ -65,4 +71,15 @@ class PageIncludeContent { return $this->contents; } + + public function toHtml(): string + { + $html = ''; + + foreach ($this->contents as $content) { + $html .= $content->ownerDocument->saveHTML($content); + } + + return $html; + } } diff --git a/app/Entities/Tools/PageIncludeParser.php b/app/Entities/Tools/PageIncludeParser.php index 02af3fce9..f1fbfba03 100644 --- a/app/Entities/Tools/PageIncludeParser.php +++ b/app/Entities/Tools/PageIncludeParser.php @@ -19,6 +19,9 @@ class PageIncludeParser */ protected array $toCleanup = []; + /** + * @param Closure(PageIncludeTag $tag): PageContent $pageContentForId + */ public function __construct( protected HtmlDocument $doc, protected Closure $pageContentForId, @@ -35,8 +38,8 @@ class PageIncludeParser $tags = $this->locateAndIsolateIncludeTags(); foreach ($tags as $tag) { - $htmlContent = $this->pageContentForId->call($this, $tag->getPageId()); - $content = new PageIncludeContent($htmlContent, $tag); + /** @var PageIncludeContent $content */ + $content = $this->pageContentForId->call($this, $tag); if (!$content->isInline()) { $parentP = $this->getParentParagraph($tag->domNode); diff --git a/app/Theming/ThemeEvents.php b/app/Theming/ThemeEvents.php index 9e14707de..3d8cd4167 100644 --- a/app/Theming/ThemeEvents.php +++ b/app/Theming/ThemeEvents.php @@ -2,8 +2,6 @@ namespace BookStack\Theming; -use BookStack\Entities\Models\Page; - /** * The ThemeEvents used within BookStack. * @@ -93,8 +91,8 @@ class ThemeEvents * * @param string $tagReference * @param string $replacementHTML - * @param Page $currentPage - * @param ?Page $referencedPage + * @param \BookStack\Entities\Models\Page $currentPage + * @param ?\BookStack\Entities\Models\Page $referencedPage */ const PAGE_INCLUDE_PARSE = 'page_include_parse'; diff --git a/app/Theming/ThemeService.php b/app/Theming/ThemeService.php index 31a7d3c64..0c2526536 100644 --- a/app/Theming/ThemeService.php +++ b/app/Theming/ThemeService.php @@ -48,6 +48,14 @@ class ThemeService return null; } + /** + * Check if there are listeners registered for the given event name. + */ + public function hasListeners(string $event): bool + { + return count($this->listeners[$event] ?? []) > 0; + } + /** * Register a new custom artisan command to be available. */ diff --git a/tests/Entity/PageContentTest.php b/tests/Entity/PageContentTest.php index 5b46c08a3..958598fda 100644 --- a/tests/Entity/PageContentTest.php +++ b/tests/Entity/PageContentTest.php @@ -88,6 +88,19 @@ class PageContentTest extends TestCase $this->withHtml($pageResp)->assertElementNotContains('#bkmrk-test', 'Hello Barry Hello Barry Hello Barry Hello Barry Hello Barry ' . $tag); } + public function test_page_includes_to_nonexisting_pages_does_not_error() + { + $page = $this->entities->page(); + $missingId = Page::query()->max('id') + 1; + $tag = "{{@{$missingId}}}"; + $page->html = '

Hello Barry ' . $tag . '

'; + $page->save(); + + $pageResp = $this->asEditor()->get($page->getUrl()); + $pageResp->assertOk(); + $pageResp->assertSee('Hello Barry'); + } + public function test_page_content_scripts_removed_by_default() { $this->asEditor(); diff --git a/tests/Unit/PageIncludeParserTest.php b/tests/Unit/PageIncludeParserTest.php index fc071cf79..83fded436 100644 --- a/tests/Unit/PageIncludeParserTest.php +++ b/tests/Unit/PageIncludeParserTest.php @@ -2,7 +2,9 @@ namespace Tests\Unit; +use BookStack\Entities\Tools\PageIncludeContent; use BookStack\Entities\Tools\PageIncludeParser; +use BookStack\Entities\Tools\PageIncludeTag; use BookStack\Util\HtmlDocument; use Tests\TestCase; @@ -227,8 +229,9 @@ class PageIncludeParserTest extends TestCase protected function runParserTest(string $html, array $contentById, string $expected): void { $doc = new HtmlDocument($html); - $parser = new PageIncludeParser($doc, function (int $id) use ($contentById) { - return $contentById[strval($id)] ?? ''; + $parser = new PageIncludeParser($doc, function (PageIncludeTag $tag) use ($contentById): PageIncludeContent { + $html = $contentById[strval($tag->getPageId())] ?? ''; + return PageIncludeContent::fromHtmlAndTag($html, $tag); }); $parser->parse();