Skip to content

Commit 71c93c8

Browse files
ssddanbrownssddanbrown
committed
Includes: Switched page to new system
- Added mulit-level depth parsing. - Updating usage of HTML doc in page content to be efficient. - Removed now redundant PageContentTest cases. - Made some include system fixes based upon testing.
1 parent 4874dc1 commit 71c93c8

File tree

7 files changed

+58
-146
lines changed

7 files changed

+58
-146
lines changed

‎app/Entities/Tools/PageContent.php‎

Lines changed: 21 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -275,21 +275,33 @@ protected function toPlainText(): string
275275
*/
276276
public function render(bool $blankIncludes = false): string
277277
{
278-
$content = $this->page->html ?? '';
278+
$html = $this->page->html ?? '';
279279

280-
if (!config('app.allow_content_scripts')) {
281-
$content = HtmlContentFilter::removeScripts($content);
280+
if (empty($html)) {
281+
return $html;
282282
}
283283

284-
if ($blankIncludes) {
285-
$content = $this->blankPageIncludes($content);
286-
} else {
287-
for ($includeDepth = 0; $includeDepth < 3; $includeDepth++) {
288-
$content = $this->parsePageIncludes($content);
284+
$doc = new HtmlDocument($html);
285+
286+
$contentProvider = function (int $id) use ($blankIncludes) {
287+
if ($blankIncludes) {
288+
return '';
289289
}
290+
return Page::visible()->find($id)->html ?? '';
291+
};
292+
293+
$parser = new PageIncludeParser($doc, $contentProvider);
294+
$nodesAdded = 1;
295+
296+
for ($includeDepth = 0; $includeDepth < 3 && $nodesAdded !== 0; $includeDepth++) {
297+
$nodesAdded = $parser->parse();
298+
}
299+
300+
if (!config('app.allow_content_scripts')) {
301+
HtmlContentFilter::removeScriptsFromDocument($doc);
290302
}
291303

292-
return $content;
304+
return $doc->getBodyInnerHtml();
293305
}
294306

295307
/**
@@ -337,83 +349,4 @@ protected function headerNodesToLevelList(DOMNodeList $nodeList): array
337349

338350
return $tree->toArray();
339351
}
340-
341-
/**
342-
* Remove any page include tags within the given HTML.
343-
*/
344-
protected function blankPageIncludes(string $html): string
345-
{
346-
return preg_replace("/{{@\s?([0-9].*?)}}/", '', $html);
347-
}
348-
349-
/**
350-
* Parse any include tags "{{@<page_id>#section}}" to be part of the page.
351-
*/
352-
protected function parsePageIncludes(string $html): string
353-
{
354-
$matches = [];
355-
preg_match_all("/{{@\s?([0-9].*?)}}/", $html, $matches);
356-
357-
foreach ($matches[1] as $index => $includeId) {
358-
$fullMatch = $matches[0][$index];
359-
$splitInclude = explode('#', $includeId, 2);
360-
361-
// Get page id from reference
362-
$pageId = intval($splitInclude[0]);
363-
if (is_nan($pageId)) {
364-
continue;
365-
}
366-
367-
// Find page to use, and default replacement to empty string for non-matches.
368-
/** @var ?Page $matchedPage */
369-
$matchedPage = Page::visible()->find($pageId);
370-
$replacement = '';
371-
372-
if ($matchedPage && count($splitInclude) === 1) {
373-
// If we only have page id, just insert all page html and continue.
374-
$replacement = $matchedPage->html;
375-
} elseif ($matchedPage && count($splitInclude) > 1) {
376-
// Otherwise, if our include tag defines a section, load that specific content
377-
$innerContent = $this->fetchSectionOfPage($matchedPage, $splitInclude[1]);
378-
$replacement = trim($innerContent);
379-
}
380-
381-
$themeReplacement = Theme::dispatch(
382-
ThemeEvents::PAGE_INCLUDE_PARSE,
383-
$includeId,
384-
$replacement,
385-
clone $this->page,
386-
$matchedPage ? (clone $matchedPage) : null,
387-
);
388-
389-
// Perform the content replacement
390-
$html = str_replace($fullMatch, $themeReplacement ?? $replacement, $html);
391-
}
392-
393-
return $html;
394-
}
395-
396-
/**
397-
* Fetch the content from a specific section of the given page.
398-
*/
399-
protected function fetchSectionOfPage(Page $page, string $sectionId): string
400-
{
401-
$topLevelTags = ['table', 'ul', 'ol', 'pre'];
402-
$doc = new HtmlDocument($page->html);
403-
404-
// Search included content for the id given and blank out if not exists.
405-
$matchingElem = $doc->getElementById($sectionId);
406-
if ($matchingElem === null) {
407-
return '';
408-
}
409-
410-
// Otherwise replace the content with the found content
411-
// Checks if the top-level wrapper should be included by matching on tag types
412-
$isTopLevel = in_array(strtolower($matchingElem->nodeName), $topLevelTags);
413-
if ($isTopLevel) {
414-
return $doc->getNodeOuterHtml($matchingElem);
415-
}
416-
417-
return $doc->getNodeInnerHtml($matchingElem);
418-
}
419352
}

‎app/Entities/Tools/PageIncludeContent.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ class PageIncludeContent
1414
*/
1515
protected array $contents = [];
1616

17-
protected bool $isTopLevel;
17+
protected bool $isTopLevel = false;
1818

1919
public function __construct(
2020
string $html,

‎app/Entities/Tools/PageIncludeParser.php‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,19 @@ class PageIncludeParser
2020
protected array $toCleanup = [];
2121

2222
public function __construct(
23-
protected string $pageHtml,
23+
protected HtmlDocument $doc,
2424
protected Closure $pageContentForId,
2525
) {
2626
}
2727

2828
/**
2929
* Parse out the include tags.
30+
* Returns the count of new content DOM nodes added to the document.
3031
*/
31-
public function parse(): string
32+
public function parse(): int
3233
{
33-
$doc = new HtmlDocument($this->pageHtml);
34-
35-
$tags = $this->locateAndIsolateIncludeTags($doc);
34+
$nodesAdded = 0;
35+
$tags = $this->locateAndIsolateIncludeTags();
3636

3737
foreach ($tags as $tag) {
3838
$htmlContent = $this->pageContentForId->call($this, $tag->getPageId());
@@ -48,22 +48,24 @@ public function parse(): string
4848
}
4949
}
5050

51-
$this->replaceNodeWithNodes($tag->domNode, $content->toDomNodes());
51+
$replacementNodes = $content->toDomNodes();
52+
$nodesAdded += count($replacementNodes);
53+
$this->replaceNodeWithNodes($tag->domNode, $replacementNodes);
5254
}
5355

5456
$this->cleanup();
5557

56-
return $doc->getBodyInnerHtml();
58+
return $nodesAdded;
5759
}
5860

5961
/**
6062
* Locate include tags within the given document, isolating them to their
6163
* own nodes in the DOM for future targeted manipulation.
6264
* @return PageIncludeTag[]
6365
*/
64-
protected function locateAndIsolateIncludeTags(HtmlDocument $doc): array
66+
protected function locateAndIsolateIncludeTags(): array
6567
{
66-
$includeHosts = $doc->queryXPath("//body//*[text()[contains(., '{{@')]]");
68+
$includeHosts = $this->doc->queryXPath("//*[text()[contains(., '{{@')]]");
6769
$includeTags = [];
6870

6971
/** @var DOMNode $node */
@@ -125,7 +127,7 @@ protected function replaceNodeWithNodes(DOMNode $toReplace, array $replacements)
125127

126128
foreach ($replacements as $replacement) {
127129
if ($replacement->ownerDocument !== $targetDoc) {
128-
$replacement = $targetDoc->adoptNode($replacement);
130+
$replacement = $targetDoc->importNode($replacement, true);
129131
}
130132

131133
$toReplace->parentNode->insertBefore($replacement, $toReplace);
@@ -190,7 +192,7 @@ protected function getParentParagraph(DOMNode $parent): ?DOMNode
190192
return $parent;
191193
}
192194

193-
$parent = $parent->parentElement;
195+
$parent = $parent->parentNode;
194196
} while ($parent !== null);
195197

196198
return null;

‎app/Theming/CustomHtmlHeadContentProvider.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public function forExport(): string
5050
$hash = md5($content);
5151

5252
return $this->cache->remember('custom-head-export:' . $hash, 86400, function () use ($content) {
53-
return HtmlContentFilter::removeScripts($content);
53+
return HtmlContentFilter::removeScriptsFromHtmlString($content);
5454
});
5555
}
5656

‎app/Util/HtmlContentFilter.php‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,10 @@
99
class HtmlContentFilter
1010
{
1111
/**
12-
* Remove all the script elements from the given HTML.
12+
* Remove all the script elements from the given HTML document.
1313
*/
14-
public static function removeScripts(string $html): string
14+
public static function removeScriptsFromDocument(HtmlDocument $doc)
1515
{
16-
if (empty($html)) {
17-
return $html;
18-
}
19-
20-
$doc = new HtmlDocument($html);
21-
2216
// Remove standard script tags
2317
$scriptElems = $doc->queryXPath('//script');
2418
static::removeNodes($scriptElems);
@@ -53,6 +47,19 @@ public static function removeScripts(string $html): string
5347
// Remove 'on*' attributes
5448
$onAttributes = $doc->queryXPath('//@*[starts-with(name(), \'on\')]');
5549
static::removeAttributes($onAttributes);
50+
}
51+
52+
/**
53+
* Remove scripts from the given HTML string.
54+
*/
55+
public static function removeScriptsFromHtmlString(string $html): string
56+
{
57+
if (empty($html)) {
58+
return $html;
59+
}
60+
61+
$doc = new HtmlDocument($html);
62+
static::removeScriptsFromDocument($doc);
5663

5764
return $doc->getBodyInnerHtml();
5865
}

‎tests/Entity/PageContentTest.php‎

Lines changed: 1 addition & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
class PageContentTest extends TestCase
1010
{
11-
protected $base64Jpeg = '/9j/2wBDAAMCAgICAgMCAgIDAwMDBAYEBAQEBAgGBgUGCQgKCgkICQkKDA8MCgsOCwkJDRENDg8QEBEQCgwSExIQEw8QEBD/yQALCAABAAEBAREA/8wABgAQEAX/2gAIAQEAAD8A0s8g/9k=';
11+
protected string $base64Jpeg = '/9j/2wBDAAMCAgICAgMCAgIDAwMDBAYEBAQEBAgGBgUGCQgKCgkICQkKDA8MCgsOCwkJDRENDg8QEBEQCgwSExIQEw8QEBD/yQALCAABAAEBAREA/8wABgAQEAX/2gAIAQEAAD8A0s8g/9k=';
1212

1313
public function test_page_includes()
1414
{
@@ -57,38 +57,6 @@ public function test_saving_page_with_includes()
5757
$this->assertEquals('', $page->text);
5858
}
5959

60-
public function test_page_includes_do_not_break_tables()
61-
{
62-
$page = $this->entities->page();
63-
$secondPage = $this->entities->page();
64-
65-
$content = '<table id="table"><tbody><tr><td>test</td></tr></tbody></table>';
66-
$secondPage->html = $content;
67-
$secondPage->save();
68-
69-
$page->html = "{{@{$secondPage->id}#table}}";
70-
$page->save();
71-
72-
$pageResp = $this->asEditor()->get($page->getUrl());
73-
$pageResp->assertSee($content, false);
74-
}
75-
76-
public function test_page_includes_do_not_break_code()
77-
{
78-
$page = $this->entities->page();
79-
$secondPage = $this->entities->page();
80-
81-
$content = '<pre id="bkmrk-code"><code>var cat = null;</code></pre>';
82-
$secondPage->html = $content;
83-
$secondPage->save();
84-
85-
$page->html = "{{@{$secondPage->id}#bkmrk-code}}";
86-
$page->save();
87-
88-
$pageResp = $this->asEditor()->get($page->getUrl());
89-
$pageResp->assertSee($content, false);
90-
}
91-
9260
public function test_page_includes_rendered_on_book_export()
9361
{
9462
$page = $this->entities->page();

‎tests/Unit/PageIncludeParserTest.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace Tests\Unit;
44

55
use BookStack\Entities\Tools\PageIncludeParser;
6+
use BookStack\Util\HtmlDocument;
67
use Tests\TestCase;
78

89
class PageIncludeParserTest extends TestCase
@@ -214,13 +215,14 @@ public function test_multiple_tags_in_shallow_origin_with_multi_block_content()
214215
);
215216
}
216217

217-
protected function runParserTest(string $html, array $contentById, string $expected)
218+
protected function runParserTest(string $html, array $contentById, string $expected): void
218219
{
219-
$parser = new PageIncludeParser($html, function (int $id) use ($contentById) {
220+
$doc = new HtmlDocument($html);
221+
$parser = new PageIncludeParser($doc, function (int $id) use ($contentById) {
220222
return $contentById[strval($id)] ?? '';
221223
});
222224

223-
$result = $parser->parse();
224-
$this->assertEquals($expected, $result);
225+
$parser->parse();
226+
$this->assertEquals($expected, $doc->getBodyInnerHtml());
225227
}
226228
}

0 commit comments

Comments
 (0)