From c9aa1c979f7689aea6bae6ad7c2f2b75f45f4c0e Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 22 May 2022 11:52:42 +0100 Subject: [PATCH 1/8] Added SVG support to the image gallery. --- app/Http/Controllers/Controller.php | 2 +- app/Uploads/ImageService.php | 14 +++++++++++--- tests/Uploads/ImageTest.php | 17 +++++++++++++++++ tests/Uploads/UsesImages.php | 9 ++++----- tests/test-data/diagram.svg | 3 +++ 5 files changed, 36 insertions(+), 9 deletions(-) create mode 100644 tests/test-data/diagram.svg diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php index 5b2221fc123..01911808f47 100644 --- a/app/Http/Controllers/Controller.php +++ b/app/Http/Controllers/Controller.php @@ -219,6 +219,6 @@ protected function logActivity(string $type, $detail = ''): void */ protected function getImageValidationRules(): array { - return ['image_extension', 'mimes:jpeg,png,gif,webp', 'max:' . (config('app.upload_limit') * 1000)]; + return ['image_extension', 'mimes:jpeg,png,gif,webp,svg', 'max:' . (config('app.upload_limit') * 1000)]; } } diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php index ca0db997b47..b5e048892a2 100644 --- a/app/Uploads/ImageService.php +++ b/app/Uploads/ImageService.php @@ -30,7 +30,7 @@ class ImageService protected $image; protected $fileSystem; - protected static $supportedExtensions = ['jpg', 'jpeg', 'png', 'gif', 'webp']; + protected static $supportedExtensions = ['jpg', 'jpeg', 'png', 'gif', 'webp', 'svg']; /** * ImageService constructor. @@ -230,6 +230,14 @@ protected function isGif(Image $image): bool return strtolower(pathinfo($image->path, PATHINFO_EXTENSION)) === 'gif'; } + /** + * Check if the given image is an SVG image file. + */ + protected function isSvg(Image $image): bool + { + return strtolower(pathinfo($image->path, PATHINFO_EXTENSION)) === 'svg'; + } + /** * Check if the given image and image data is apng. */ @@ -255,8 +263,8 @@ protected function isApngData(Image $image, string &$imageData): bool */ public function getThumbnail(Image $image, ?int $width, ?int $height, bool $keepRatio = false): string { - // Do not resize GIF images where we're not cropping - if ($keepRatio && $this->isGif($image)) { + // Do not resize GIF images where we're not cropping or SVG images. + if (($keepRatio && $this->isGif($image)) || $this->isSvg($image)) { return $this->getPublicUrl($image->path); } diff --git a/tests/Uploads/ImageTest.php b/tests/Uploads/ImageTest.php index 01754d2dec1..4188968289a 100644 --- a/tests/Uploads/ImageTest.php +++ b/tests/Uploads/ImageTest.php @@ -74,6 +74,23 @@ public function test_image_display_thumbnail_generation_for_apng_images_uses_ori $this->assertStringNotContainsString('thumbs-', $imgDetails['response']->thumbs->display); } + public function test_svg_upload() + { + /** @var Page $page */ + $page = Page::query()->first(); + $admin = $this->getAdmin(); + $this->actingAs($admin); + + $imgDetails = $this->uploadGalleryImage($page, 'diagram.svg', 'image/svg+xml'); + $this->assertFileExists(public_path($imgDetails['path'])); + $this->assertTrue( + $imgDetails['response']->url === $imgDetails['response']->thumbs->gallery + && $imgDetails['response']->url === $imgDetails['response']->thumbs->display, + ); + + $this->deleteImage($imgDetails['path']); + } + public function test_image_edit() { $editor = $this->getEditor(); diff --git a/tests/Uploads/UsesImages.php b/tests/Uploads/UsesImages.php index b55572248a8..513a5d49e9a 100644 --- a/tests/Uploads/UsesImages.php +++ b/tests/Uploads/UsesImages.php @@ -3,6 +3,7 @@ namespace Tests\Uploads; use BookStack\Entities\Models\Page; +use BookStack\Uploads\Image; use Illuminate\Http\UploadedFile; use stdClass; @@ -84,21 +85,19 @@ protected function uploadImage($name, $uploadedTo = 0, $contentType = 'image/png * Returns the image name. * Can provide a page to relate the image to. * - * @param Page|null $page - * * @return array{name: string, path: string, page: Page, response: stdClass} */ - protected function uploadGalleryImage(Page $page = null, ?string $testDataFileName = null) + protected function uploadGalleryImage(Page $page = null, string $testDataFileName = 'first-image.png', string $contentType = 'image/png') { if ($page === null) { $page = Page::query()->first(); } - $imageName = $testDataFileName ?? 'first-image.png'; + $imageName = $testDataFileName; $relPath = $this->getTestImagePath('gallery', $imageName); $this->deleteImage($relPath); - $upload = $this->uploadImage($imageName, $page->id, 'image/png', $testDataFileName); + $upload = $this->uploadImage($imageName, $page->id, $contentType, $testDataFileName); $upload->assertStatus(200); return [ diff --git a/tests/test-data/diagram.svg b/tests/test-data/diagram.svg new file mode 100644 index 00000000000..75e3a49e082 --- /dev/null +++ b/tests/test-data/diagram.svg @@ -0,0 +1,3 @@ + + +
Hello!
Hello!Text is not SVG - cannot display \ No newline at end of file From b69722c3b5aa0c64f46c97f4ba6e4532387e711b Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 22 May 2022 11:58:22 +0100 Subject: [PATCH 2/8] Fixed issue caused by changing test method defaults --- tests/Uploads/UsesImages.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/Uploads/UsesImages.php b/tests/Uploads/UsesImages.php index 513a5d49e9a..3140d0cd296 100644 --- a/tests/Uploads/UsesImages.php +++ b/tests/Uploads/UsesImages.php @@ -40,9 +40,9 @@ protected function newTestImageFromBase64(string $base64FileName, $imageFileName /** * Get a test image that can be uploaded. */ - protected function getTestImage(string $fileName, ?string $testDataFileName = null): UploadedFile + protected function getTestImage(string $fileName, ?string $testDataFileName = null, $mimeType = 'image/png'): UploadedFile { - return new UploadedFile($this->getTestImageFilePath($testDataFileName), $fileName, 'image/png', null, true); + return new UploadedFile($this->getTestImageFilePath($testDataFileName), $fileName, $mimeType, null, true); } /** @@ -74,7 +74,7 @@ protected function getTestImagePath(string $type, string $fileName): string */ protected function uploadImage($name, $uploadedTo = 0, $contentType = 'image/png', ?string $testDataFileName = null) { - $file = $this->getTestImage($name, $testDataFileName); + $file = $this->getTestImage($name, $testDataFileName, $contentType); return $this->withHeader('Content-Type', $contentType) ->call('POST', '/images/gallery', ['uploaded_to' => $uploadedTo], [], ['file' => $file], []); @@ -87,13 +87,13 @@ protected function uploadImage($name, $uploadedTo = 0, $contentType = 'image/png * * @return array{name: string, path: string, page: Page, response: stdClass} */ - protected function uploadGalleryImage(Page $page = null, string $testDataFileName = 'first-image.png', string $contentType = 'image/png') + protected function uploadGalleryImage(Page $page = null, string $testDataFileName = null, string $contentType = 'image/png') { if ($page === null) { $page = Page::query()->first(); } - $imageName = $testDataFileName; + $imageName = $testDataFileName ?? 'first-image.png'; $relPath = $this->getTestImagePath('gallery', $imageName); $this->deleteImage($relPath); From d926ca5f71314c40058932ac7886c5fe9e2c8a6b Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 22 May 2022 12:38:50 +0100 Subject: [PATCH 3/8] Updated draw.io code to support SVGs as primary data type --- .../Images/DrawioImageController.php | 5 +- app/Uploads/ImageRepo.php | 3 +- resources/js/components/markdown-editor.js | 8 +-- resources/js/services/drawio.js | 4 +- resources/js/wysiwyg/plugin-drawio.js | 6 +-- tests/Uploads/DrawioTest.php | 52 +++++++++++++++++-- 6 files changed, 63 insertions(+), 15 deletions(-) diff --git a/app/Http/Controllers/Images/DrawioImageController.php b/app/Http/Controllers/Images/DrawioImageController.php index cab1c925e41..31c375c25be 100644 --- a/app/Http/Controllers/Images/DrawioImageController.php +++ b/app/Http/Controllers/Images/DrawioImageController.php @@ -76,8 +76,11 @@ public function getAsBase64($id) return $this->jsonError('Image data could not be found'); } + $isSvg = strtolower(pathinfo($image->path, PATHINFO_EXTENSION)) === 'svg'; + $uriPrefix = $isSvg ? 'data:image/svg+xml;base64,' : 'data:image/png;base64,'; + return response()->json([ - 'content' => base64_encode($imageData), + 'content' => $uriPrefix . base64_encode($imageData), ]); } } diff --git a/app/Uploads/ImageRepo.php b/app/Uploads/ImageRepo.php index bfe4b597739..24cd8c33d0c 100644 --- a/app/Uploads/ImageRepo.php +++ b/app/Uploads/ImageRepo.php @@ -148,7 +148,8 @@ public function saveNewFromData(string $imageName, string $imageData, string $ty */ public function saveDrawing(string $base64Uri, int $uploadedTo): Image { - $name = 'Drawing-' . user()->id . '-' . time() . '.png'; + $isSvg = strpos($base64Uri, 'data:image/svg+xml;') === 0; + $name = 'Drawing-' . user()->id . '-' . time() . ($isSvg ? '.svg' : '.png'); return $this->imageService->saveNewFromBase64Uri($base64Uri, $name, 'drawio', $uploadedTo); } diff --git a/resources/js/components/markdown-editor.js b/resources/js/components/markdown-editor.js index 297d9c8ece8..71a56af5520 100644 --- a/resources/js/components/markdown-editor.js +++ b/resources/js/components/markdown-editor.js @@ -445,10 +445,10 @@ class MarkdownEditor { DrawIO.show(url,() => { return Promise.resolve(''); - }, (pngData) => { + }, (drawingData) => { const data = { - image: pngData, + image: drawingData, uploaded_to: Number(this.pageId), }; @@ -480,10 +480,10 @@ class MarkdownEditor { DrawIO.show(drawioUrl, () => { return DrawIO.load(drawingId); - }, (pngData) => { + }, (drawingData) => { let data = { - image: pngData, + image: drawingData, uploaded_to: Number(this.pageId), }; diff --git a/resources/js/services/drawio.js b/resources/js/services/drawio.js index dfca832117f..141b61c72d4 100644 --- a/resources/js/services/drawio.js +++ b/resources/js/services/drawio.js @@ -55,7 +55,7 @@ function drawEventExport(message) { } function drawEventSave(message) { - drawPostMessage({action: 'export', format: 'xmlpng', xml: message.xml, spin: 'Updating drawing'}); + drawPostMessage({action: 'export', format: 'xmlsvg', xml: message.xml, spin: 'Updating drawing'}); } function drawEventInit() { @@ -96,7 +96,7 @@ async function upload(imageData, pageUploadedToId) { */ async function load(drawingId) { const resp = await window.$http.get(window.baseUrl(`/images/drawio/base64/${drawingId}`)); - return `data:image/png;base64,${resp.data.content}`; + return resp.data.content; } export default {show, close, upload, load}; \ No newline at end of file diff --git a/resources/js/wysiwyg/plugin-drawio.js b/resources/js/wysiwyg/plugin-drawio.js index 6b0167a7045..c270c5d20f9 100644 --- a/resources/js/wysiwyg/plugin-drawio.js +++ b/resources/js/wysiwyg/plugin-drawio.js @@ -34,7 +34,7 @@ function showDrawingEditor(mceEditor, selectedNode = null) { DrawIO.show(options.drawioUrl, drawingInit, updateContent); } -async function updateContent(pngData) { +async function updateContent(drawingData) { const id = "image-" + Math.random().toString(16).slice(2); const loadingImage = window.baseUrl('/loading.gif'); @@ -52,7 +52,7 @@ async function updateContent(pngData) { DrawIO.close(); let imgElem = currentNode.querySelector('img'); try { - const img = await DrawIO.upload(pngData, options.pageId); + const img = await DrawIO.upload(drawingData, options.pageId); pageEditor.dom.setAttrib(imgElem, 'src', img.url); pageEditor.dom.setAttrib(currentNode, 'drawio-diagram', img.id); } catch (err) { @@ -65,7 +65,7 @@ async function updateContent(pngData) { pageEditor.insertContent(`
`); DrawIO.close(); try { - const img = await DrawIO.upload(pngData, options.pageId); + const img = await DrawIO.upload(drawingData, options.pageId); pageEditor.dom.setAttrib(id, 'src', img.url); pageEditor.dom.get(id).parentNode.setAttribute('drawio-diagram', img.id); } catch (err) { diff --git a/tests/Uploads/DrawioTest.php b/tests/Uploads/DrawioTest.php index 2ed4da7cadc..232904275f6 100644 --- a/tests/Uploads/DrawioTest.php +++ b/tests/Uploads/DrawioTest.php @@ -10,7 +10,7 @@ class DrawioTest extends TestCase { use UsesImages; - public function test_get_image_as_base64() + public function test_get_image_as_base64_with_png_content() { $page = Page::first(); $this->asAdmin(); @@ -23,11 +23,27 @@ public function test_get_image_as_base64() $imageGet = $this->getJson("/images/drawio/base64/{$image->id}"); $imageGet->assertJson([ - 'content' => 'iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAIAAAACDbGyAAAACXBIWXMAAAsTAAALEwEAmpwYAAAAB3RJTUUH4gEcDCo5iYNs+gAAAB1pVFh0Q29tbWVudAAAAAAAQ3JlYXRlZCB3aXRoIEdJTVBkLmUHAAAAFElEQVQI12O0jN/KgASYGFABqXwAZtoBV6Sl3hIAAAAASUVORK5CYII=', + 'content' => 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAIAAAACDbGyAAAACXBIWXMAAAsTAAALEwEAmpwYAAAAB3RJTUUH4gEcDCo5iYNs+gAAAB1pVFh0Q29tbWVudAAAAAAAQ3JlYXRlZCB3aXRoIEdJTVBkLmUHAAAAFElEQVQI12O0jN/KgASYGFABqXwAZtoBV6Sl3hIAAAAASUVORK5CYII=', ]); } - public function test_drawing_base64_upload() + public function test_get_image_as_base64_with_svg_content() + { + $page = Page::first(); + $this->asAdmin(); + + $this->uploadImage('my-drawing.svg', $page->id, 'image/svg+xml', 'diagram.svg'); + $image = Image::first(); + $image->type = 'drawio'; + $image->save(); + + $imageGet = $this->getJson("/images/drawio/base64/{$image->id}"); + $imageGet->assertJson([ + 'content' => 'data:image/svg+xml;base64,' . base64_encode(file_get_contents($this->getTestImageFilePath('diagram.svg'))), + ]); + } + + public function test_drawing_base64_upload_with_png() { $page = Page::first(); $editor = $this->getEditor(); @@ -35,7 +51,7 @@ public function test_drawing_base64_upload() $upload = $this->postJson('images/drawio', [ 'uploaded_to' => $page->id, - 'image' => 'image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAIAAAACDbGyAAAACXBIWXMAAAsTAAALEwEAmpwYAAAAB3RJTUUH4gEcDCo5iYNs+gAAAB1pVFh0Q29tbWVudAAAAAAAQ3JlYXRlZCB3aXRoIEdJTVBkLmUHAAAAFElEQVQI12O0jN/KgASYGFABqXwAZtoBV6Sl3hIAAAAASUVORK5CYII=', + 'image' => 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAIAAAACDbGyAAAACXBIWXMAAAsTAAALEwEAmpwYAAAAB3RJTUUH4gEcDCo5iYNs+gAAAB1pVFh0Q29tbWVudAAAAAAAQ3JlYXRlZCB3aXRoIEdJTVBkLmUHAAAAFElEQVQI12O0jN/KgASYGFABqXwAZtoBV6Sl3hIAAAAASUVORK5CYII=', ]); $upload->assertStatus(200); @@ -54,6 +70,34 @@ public function test_drawing_base64_upload() $this->assertTrue($testImageData === $uploadedImageData, 'Uploaded image file data does not match our test image as expected'); } + public function test_drawing_base64_upload_with_svg() + { + $page = Page::first(); + $editor = $this->getEditor(); + $this->actingAs($editor); + + $upload = $this->postJson('images/drawio', [ + 'uploaded_to' => $page->id, + 'image' => 'data:image/svg+xml;base64,' . base64_encode(file_get_contents($this->getTestImageFilePath('diagram.svg'))), + ]); + + $upload->assertStatus(200); + $upload->assertJson([ + 'type' => 'drawio', + 'uploaded_to' => $page->id, + 'created_by' => $editor->id, + 'updated_by' => $editor->id, + ]); + + $image = Image::where('type', '=', 'drawio')->first(); + $this->assertStringEndsWith('.svg', $image->path); + $this->assertTrue(file_exists(public_path($image->path)), 'Uploaded image not found at path: ' . public_path($image->path)); + + $testImageData = file_get_contents($this->getTestImageFilePath('diagram.svg')); + $uploadedImageData = file_get_contents(public_path($image->path)); + $this->assertTrue($testImageData === $uploadedImageData, 'Uploaded image file data does not match our test image as expected'); + } + public function test_drawio_url_can_be_configured() { config()->set('services.drawio', 'http://cats.com?dog=tree'); From 5fd8e7e0e98d97337daadc18ebe88c6f28b0c18c Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 23 May 2022 12:24:40 +0100 Subject: [PATCH 4/8] Updated drawio tinymce plugin to use embeds - Adds support for handling drawings as embeds, based on image extension. - Adds additional attribute to drawio elements within editor to prevent tinymce replacing embeds with a placeholder. - Updates how contenteditable is applied to drawio blocks within editor, to use proper filters instead of using the SetContent event. --- resources/js/services/drawio.js | 16 +++++++- resources/js/wysiwyg/plugin-drawio.js | 55 ++++++++++++++++++--------- resources/sass/_tinymce.scss | 5 +++ 3 files changed, 56 insertions(+), 20 deletions(-) diff --git a/resources/js/services/drawio.js b/resources/js/services/drawio.js index 141b61c72d4..78fb968cf4b 100644 --- a/resources/js/services/drawio.js +++ b/resources/js/services/drawio.js @@ -99,4 +99,18 @@ async function load(drawingId) { return resp.data.content; } -export default {show, close, upload, load}; \ No newline at end of file + +function buildDrawingContentHtml(drawing) { + const isSvg = drawing.url.split('.').pop().toLowerCase() === 'svg'; + const image = ``; + const embed = ``; + return `
${isSvg ? embed : image}
` +} + +function buildDrawingContentNode(drawing) { + const div = document.createElement('div'); + div.innerHTML = buildDrawingContentHtml(drawing); + return div.children[0]; +} + +export default {show, close, upload, load, buildDrawingContentHtml, buildDrawingContentNode}; \ No newline at end of file diff --git a/resources/js/wysiwyg/plugin-drawio.js b/resources/js/wysiwyg/plugin-drawio.js index c270c5d20f9..58dff661f93 100644 --- a/resources/js/wysiwyg/plugin-drawio.js +++ b/resources/js/wysiwyg/plugin-drawio.js @@ -1,4 +1,5 @@ import DrawIO from "../services/drawio"; +import {build} from "./config"; let pageEditor = null; let currentNode = null; @@ -15,15 +16,14 @@ function isDrawing(node) { function showDrawingManager(mceEditor, selectedNode = null) { pageEditor = mceEditor; currentNode = selectedNode; + // Show image manager window.ImageManager.show(function (image) { if (selectedNode) { - let imgElem = selectedNode.querySelector('img'); - pageEditor.dom.setAttrib(imgElem, 'src', image.url); - pageEditor.dom.setAttrib(selectedNode, 'drawio-diagram', image.id); + pageEditor.dom.replace(buildDrawingNode(image), selectedNode); } else { - let imgHTML = `
`; - pageEditor.insertContent(imgHTML); + const drawingHtml = DrawIO.buildDrawingContentHtml(image); + pageEditor.insertContent(drawingHtml); } }, 'drawio'); } @@ -34,6 +34,13 @@ function showDrawingEditor(mceEditor, selectedNode = null) { DrawIO.show(options.drawioUrl, drawingInit, updateContent); } +function buildDrawingNode(drawing) { + const drawingEl = DrawIO.buildDrawingContentNode(drawing); + drawingEl.setAttribute('contenteditable', 'false'); + drawingEl.setAttribute('data-ephox-embed-iri', 'true'); + return drawingEl; +} + async function updateContent(drawingData) { const id = "image-" + Math.random().toString(16).slice(2); const loadingImage = window.baseUrl('/loading.gif'); @@ -50,11 +57,9 @@ async function updateContent(drawingData) { // Handle updating an existing image if (currentNode) { DrawIO.close(); - let imgElem = currentNode.querySelector('img'); try { const img = await DrawIO.upload(drawingData, options.pageId); - pageEditor.dom.setAttrib(imgElem, 'src', img.url); - pageEditor.dom.setAttrib(currentNode, 'drawio-diagram', img.id); + pageEditor.dom.replace(buildDrawingNode(img), currentNode); } catch (err) { handleUploadError(err); } @@ -62,12 +67,11 @@ async function updateContent(drawingData) { } setTimeout(async () => { - pageEditor.insertContent(`
`); + pageEditor.insertContent(`
Loading
`); DrawIO.close(); try { const img = await DrawIO.upload(drawingData, options.pageId); - pageEditor.dom.setAttrib(id, 'src', img.url); - pageEditor.dom.get(id).parentNode.setAttribute('drawio-diagram', img.id); + pageEditor.dom.replace(buildDrawingNode(img), pageEditor.dom.get(id).parentNode); } catch (err) { pageEditor.dom.remove(id); handleUploadError(err); @@ -86,7 +90,6 @@ function drawingInit() { } /** - * * @param {WysiwygConfigOptions} providedOptions * @return {function(Editor, string)} */ @@ -130,14 +133,28 @@ export function getPlugin(providedOptions) { showDrawingEditor(editor, selectedNode); }); - editor.on('SetContent', function () { - const drawings = editor.$('body > div[drawio-diagram]'); - if (!drawings.length) return; + editor.on('PreInit', () => { + editor.parser.addNodeFilter('div', function(nodes) { + for (const node of nodes) { + if (node.attr('drawio-diagram')) { + // Set content editable to be false to prevent direct editing of child content. + node.attr('contenteditable', 'false'); + // Set this attribute to prevent drawing contents being parsed as media embeds + // to avoid contents being replaced with placeholder images. + // TinyMCE embed plugin sources looks for this attribute in its logic. + node.attr('data-ephox-embed-iri', 'true'); + } + } + }); - editor.undoManager.transact(function () { - drawings.each((index, elem) => { - elem.setAttribute('contenteditable', 'false'); - }); + editor.serializer.addNodeFilter('div', function(nodes) { + for (const node of nodes) { + // Clean up content attributes + if (node.attr('drawio-diagram')) { + node.attr('contenteditable', null); + node.attr('data-ephox-embed-iri', null); + } + } }); }); diff --git a/resources/sass/_tinymce.scss b/resources/sass/_tinymce.scss index 0ee3fa40b23..aa6b05fa605 100644 --- a/resources/sass/_tinymce.scss +++ b/resources/sass/_tinymce.scss @@ -48,6 +48,11 @@ body.page-content.mce-content-body { display: none; } +// Prevent interaction with embed contents +.page-content.mce-content-body embed { + pointer-events: none; +} + // Details/summary editor usability .page-content.mce-content-body details summary { pointer-events: none; From 641a26cdf7bcf68bae7dc38597dd539d19bf5ee7 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 23 May 2022 14:38:34 +0100 Subject: [PATCH 5/8] Updated markdown editor to use svg drawio images - Also tweaked page editor to not error when the current user does not have permission to change editor type. --- resources/js/components/markdown-editor.js | 15 ++++++++------- resources/js/components/page-editor.js | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/resources/js/components/markdown-editor.js b/resources/js/components/markdown-editor.js index 71a56af5520..8d117870c18 100644 --- a/resources/js/components/markdown-editor.js +++ b/resources/js/components/markdown-editor.js @@ -462,7 +462,7 @@ class MarkdownEditor { } insertDrawing(image, originalCursor) { - const newText = `
`; + const newText = DrawIO.buildDrawingContentHtml(image); this.cm.focus(); this.cm.replaceSelection(newText); this.cm.setCursor(originalCursor.line, originalCursor.ch + newText.length); @@ -488,13 +488,14 @@ class MarkdownEditor { }; window.$http.post("/images/drawio", data).then(resp => { - let newText = `
`; - let newContent = this.cm.getValue().split('\n').map(line => { - if (line.indexOf(`drawio-diagram="${drawingId}"`) !== -1) { - return newText; - } - return line; + const image = resp.data; + const newText = DrawIO.buildDrawingContentHtml(image); + + const newContent = this.cm.getValue().split('\n').map(line => { + const isDrawing = line.includes(`drawio-diagram="${drawingId}"`); + return isDrawing ? newText : line; }).join('\n'); + this.cm.setValue(newContent); this.cm.setCursor(cursorPos); this.cm.focus(); diff --git a/resources/js/components/page-editor.js b/resources/js/components/page-editor.js index ce123e987b0..f6e52c7073b 100644 --- a/resources/js/components/page-editor.js +++ b/resources/js/components/page-editor.js @@ -24,7 +24,7 @@ class PageEditor { this.draftDisplayIcon = this.$refs.draftDisplayIcon; this.changelogInput = this.$refs.changelogInput; this.changelogDisplay = this.$refs.changelogDisplay; - this.changeEditorButtons = this.$manyRefs.changeEditor; + this.changeEditorButtons = this.$manyRefs.changeEditor || []; this.switchDialogContainer = this.$refs.switchDialog; // Translations From 1d1186c901d200011183fa3eadd8636005884f53 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 23 May 2022 15:16:23 +0100 Subject: [PATCH 6/8] Replaced markdown preview display iframe with div No longer need to use the iframe sandboxing techniques, since we now have CSP rules in-place. Means that embed tags can load without CSP complications. Causes slight change to contents of `editor-markdown::setup` editor event data, since we're changing the `displayEl` property. Updates markdown editor component to make better use of the component system. --- resources/js/components/markdown-editor.js | 66 +++++-------------- resources/sass/_forms.scss | 20 ++---- .../pages/parts/markdown-editor.blade.php | 6 +- 3 files changed, 27 insertions(+), 65 deletions(-) diff --git a/resources/js/components/markdown-editor.js b/resources/js/components/markdown-editor.js index 8d117870c18..3f9dbeb6979 100644 --- a/resources/js/components/markdown-editor.js +++ b/resources/js/components/markdown-editor.js @@ -18,10 +18,8 @@ class MarkdownEditor { this.markdown = new MarkdownIt({html: true}); this.markdown.use(mdTasksLists, {label: true}); - this.display = this.elem.querySelector('.markdown-display'); - - this.displayStylesLoaded = false; - this.input = this.elem.querySelector('textarea'); + this.display = this.$refs.display; + this.input = this.$refs.input; this.cm = null; this.Code = null; @@ -32,23 +30,13 @@ class MarkdownEditor { }); this.onMarkdownScroll = this.onMarkdownScroll.bind(this); - - const displayLoad = () => { - this.displayDoc = this.display.contentDocument; - this.init(cmLoadPromise); - }; - - if (this.display.contentDocument.readyState === 'complete') { - displayLoad(); - } else { - this.display.addEventListener('load', displayLoad.bind(this)); - } - window.$events.emitPublic(this.elem, 'editor-markdown::setup', { markdownIt: this.markdown, displayEl: this.display, codeMirrorInstance: this.cm, }); + + this.init(cmLoadPromise); } init(cmLoadPromise) { @@ -56,17 +44,17 @@ class MarkdownEditor { let lastClick = 0; // Prevent markdown display link click redirect - this.displayDoc.addEventListener('click', event => { - let isDblClick = Date.now() - lastClick < 300; + this.display.addEventListener('click', event => { + const isDblClick = Date.now() - lastClick < 300; - let link = event.target.closest('a'); + const link = event.target.closest('a'); if (link !== null) { event.preventDefault(); window.open(link.getAttribute('href')); return; } - let drawing = event.target.closest('[drawio-diagram]'); + const drawing = event.target.closest('[drawio-diagram]'); if (drawing !== null && isDblClick) { this.actionEditDrawing(drawing); return; @@ -77,10 +65,10 @@ class MarkdownEditor { // Button actions this.elem.addEventListener('click', event => { - let button = event.target.closest('button[data-action]'); + const button = event.target.closest('button[data-action]'); if (button === null) return; - let action = button.getAttribute('data-action'); + const action = button.getAttribute('data-action'); if (action === 'insertImage') this.actionInsertImage(); if (action === 'insertLink') this.actionShowLinkSelector(); if (action === 'insertDrawing' && (event.ctrlKey || event.metaKey)) { @@ -132,35 +120,11 @@ class MarkdownEditor { window.$events.emit('editor-markdown-change', content); // Set body content - this.displayDoc.body.className = 'page-content'; - this.displayDoc.body.innerHTML = html; - - // Copy styles from page head and set custom styles for editor - this.loadStylesIntoDisplay(); - } - - loadStylesIntoDisplay() { - if (this.displayStylesLoaded) return; - this.displayDoc.documentElement.classList.add('markdown-editor-display'); - // Set display to be dark mode if parent is - - if (document.documentElement.classList.contains('dark-mode')) { - this.displayDoc.documentElement.style.backgroundColor = '#222'; - this.displayDoc.documentElement.classList.add('dark-mode'); - } - - this.displayDoc.head.innerHTML = ''; - const styles = document.head.querySelectorAll('style,link[rel=stylesheet]'); - for (let style of styles) { - const copy = style.cloneNode(true); - this.displayDoc.head.appendChild(copy); - } - - this.displayStylesLoaded = true; + this.display.innerHTML = html; } onMarkdownScroll(lineCount) { - const elems = this.displayDoc.body.children; + const elems = this.display.children; if (elems.length <= lineCount) return; const topElem = (lineCount === -1) ? elems[elems.length-1] : elems[lineCount]; @@ -317,7 +281,7 @@ class MarkdownEditor { let cursor = cm.getCursor(); let lineContent = cm.getLine(cursor.line); let lineLen = lineContent.length; - let newLineContent = lineContent; + let newLineContent; if (lineContent.indexOf(start) === 0 && lineContent.slice(-end.length) === end) { newLineContent = lineContent.slice(start.length, lineContent.length - end.length); @@ -333,9 +297,9 @@ class MarkdownEditor { let selection = cm.getSelection(); if (selection === '') return wrapLine(start, end); - let newSelection = selection; + let newSelection; let frontDiff = 0; - let endDiff = 0; + let endDiff; if (selection.indexOf(start) === 0 && selection.slice(-end.length) === end) { newSelection = selection.slice(start.length, selection.length - end.length); diff --git a/resources/sass/_forms.scss b/resources/sass/_forms.scss index 665b1213be8..40b4da89cd0 100644 --- a/resources/sass/_forms.scss +++ b/resources/sass/_forms.scss @@ -114,26 +114,20 @@ .markdown-display { margin-inline-start: -1px; -} - -.markdown-editor-display { + display: block; background-color: #fff; - body { - display: block; - background-color: #fff; - padding-inline-start: 16px; - padding-inline-end: 16px; - } + padding: $-m; + overflow-y: scroll; [drawio-diagram]:hover { outline: 2px solid var(--color-primary); } + [drawio-diagram] embed { + pointer-events: none; + } } -html.markdown-editor-display.dark-mode { +.dark-mode .markdown-display { background-color: #222; - body { - background-color: #222; - } } .editor-toolbar { diff --git a/resources/views/pages/parts/markdown-editor.blade.php b/resources/views/pages/parts/markdown-editor.blade.php index 39d628e17d0..0df17d986ab 100644 --- a/resources/views/pages/parts/markdown-editor.blade.php +++ b/resources/views/pages/parts/markdown-editor.blade.php @@ -23,6 +23,7 @@ class="flex-fill flex code-fill">
@@ -34,7 +35,10 @@ class="flex-fill flex code-fill">
{{ trans('entities.pages_md_preview') }}
- +
+
+
From 05f8034439e528fad272ab46e3698e998fa01cf1 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 23 May 2022 16:11:28 +0100 Subject: [PATCH 7/8] Added embed support for contained HTML exports Unfortunately CSP rules will block embeds anyway. Need to either relax CSP rules on exports, or instead convert to img tags? Also cleaned up existing regexes. --- app/Entities/Tools/ExportFormatter.php | 15 ++++++--------- tests/Entity/ExportTest.php | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/app/Entities/Tools/ExportFormatter.php b/app/Entities/Tools/ExportFormatter.php index 99aa4536f52..ed3e8d326f6 100644 --- a/app/Entities/Tools/ExportFormatter.php +++ b/app/Entities/Tools/ExportFormatter.php @@ -215,14 +215,13 @@ protected function replaceIframesWithLinks(string $html): string */ protected function containHtml(string $htmlContent): string { + // Replace image & embed src attributes with base64 encoded data strings $imageTagsOutput = []; - preg_match_all("/\/i", $htmlContent, $imageTagsOutput); - - // Replace image src with base64 encoded image strings + preg_match_all("/<(?:img|embed) .*?src=['\"](.*?)['\"].*?>/i", $htmlContent, $imageTagsOutput); if (isset($imageTagsOutput[0]) && count($imageTagsOutput[0]) > 0) { foreach ($imageTagsOutput[0] as $index => $imgMatch) { $oldImgTagString = $imgMatch; - $srcString = $imageTagsOutput[2][$index]; + $srcString = $imageTagsOutput[1][$index]; $imageEncoded = $this->imageService->imageUriToBase64($srcString); if ($imageEncoded === null) { $imageEncoded = $srcString; @@ -232,14 +231,13 @@ protected function containHtml(string $htmlContent): string } } + // Replace any relative links with full system URL $linksOutput = []; - preg_match_all("/\/i", $htmlContent, $linksOutput); - - // Replace image src with base64 encoded image strings + preg_match_all("//i", $htmlContent, $linksOutput); if (isset($linksOutput[0]) && count($linksOutput[0]) > 0) { foreach ($linksOutput[0] as $index => $linkMatch) { $oldLinkString = $linkMatch; - $srcString = $linksOutput[2][$index]; + $srcString = $linksOutput[1][$index]; if (strpos(trim($srcString), 'http') !== 0) { $newSrcString = url($srcString); $newLinkString = str_replace($srcString, $newSrcString, $oldLinkString); @@ -248,7 +246,6 @@ protected function containHtml(string $htmlContent): string } } - // Replace any relative links with system domain return $htmlContent; } diff --git a/tests/Entity/ExportTest.php b/tests/Entity/ExportTest.php index 08d0921111f..9debec12bdc 100644 --- a/tests/Entity/ExportTest.php +++ b/tests/Entity/ExportTest.php @@ -258,6 +258,24 @@ public function test_page_export_contained_html_does_not_allow_upward_traversal_ unlink($testFilePath); } + public function test_page_export_contained_html_embed_element_srcs_are_inlined() + { + $page = Page::query()->first(); + $page->html = ''; + $page->save(); + + $storageDisk = Storage::disk('local'); + $storageDisk->makeDirectory('uploads/images/gallery'); + $storageDisk->put('uploads/images/gallery/svg_test.svg', 'good'); + + $resp = $this->asEditor()->get($page->getUrl('/export/html')); + + $storageDisk->delete('uploads/images/gallery/svg_test.svg'); + + $resp->assertDontSee('http://localhost/uploads/images/gallery/svg_test.svg', false); + $resp->assertSee('', false); + } + public function test_exports_removes_scripts_from_custom_head() { $entities = [ From bf0ba9f756fca20d2457832e8d8adca3a2be785e Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 24 May 2022 18:05:47 +0100 Subject: [PATCH 8/8] Replaced embeds with images in exports --- app/Entities/Tools/ExportFormatter.php | 5 ++++- tests/Entity/ExportTest.php | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/Entities/Tools/ExportFormatter.php b/app/Entities/Tools/ExportFormatter.php index ed3e8d326f6..cda8e030889 100644 --- a/app/Entities/Tools/ExportFormatter.php +++ b/app/Entities/Tools/ExportFormatter.php @@ -215,9 +215,12 @@ protected function replaceIframesWithLinks(string $html): string */ protected function containHtml(string $htmlContent): string { + // Replace embed tags with images + $htmlContent = preg_replace("//i", '', $htmlContent); + // Replace image & embed src attributes with base64 encoded data strings $imageTagsOutput = []; - preg_match_all("/<(?:img|embed) .*?src=['\"](.*?)['\"].*?>/i", $htmlContent, $imageTagsOutput); + preg_match_all("//i", $htmlContent, $imageTagsOutput); if (isset($imageTagsOutput[0]) && count($imageTagsOutput[0]) > 0) { foreach ($imageTagsOutput[0] as $index => $imgMatch) { $oldImgTagString = $imgMatch; diff --git a/tests/Entity/ExportTest.php b/tests/Entity/ExportTest.php index 9debec12bdc..015fab5c364 100644 --- a/tests/Entity/ExportTest.php +++ b/tests/Entity/ExportTest.php @@ -258,7 +258,7 @@ public function test_page_export_contained_html_does_not_allow_upward_traversal_ unlink($testFilePath); } - public function test_page_export_contained_html_embed_element_srcs_are_inlined() + public function test_page_export_contained_html_embed_elements_are_converted_to_images_with_srcs_inlined() { $page = Page::query()->first(); $page->html = ''; @@ -273,7 +273,7 @@ public function test_page_export_contained_html_embed_element_srcs_are_inlined() $storageDisk->delete('uploads/images/gallery/svg_test.svg'); $resp->assertDontSee('http://localhost/uploads/images/gallery/svg_test.svg', false); - $resp->assertSee('', false); + $resp->assertSee('', false); } public function test_exports_removes_scripts_from_custom_head()