Skip to content

Add base64 image blob support - #2700

Merged
ssddanbrown merged 1 commit into
BookStackApp:masterfrom
awarre:master
Jun 2, 2021
Merged

ssddanbrown merged 1 commit into
BookStackApp:masterfrom
awarre:master

Conversation

@awarre

@awarre awarre commented Apr 21, 2021

Copy link
Copy Markdown
Contributor

Overview

This allows usage of base64 image blobs in pages by parsing them from posts during submission and saving them as images. This ties almost entirely into existing code.

Based on our discussion in #2631, I think this solution would be preferable.

Benefits
Even if this exact pull request isn't accepted, a solution with this design has many benefits.

  • Avoids storing images as text blobs in the database, instead saving them as native BookStack images.
  • Integrated directly into BookStock rather than tied to WYSIWYG editors, which could change in the future.
  • Allows importing image data via the API. Export formats from knowledge systems (including BookStack's own) often use base64 image blobs. Importing with them with the API is currently more difficult than necessary.
  • Allows copying and pasting from external sources with multiple images. The end-user doesn't need to think or know about what their datasource is or their image formats are. This is a huge benefit to usability when working with complex source information that includes images.
  • Handled server-side rather than client-side. Client-side restrictions can be easily bypassed, deliberately and accidentally.

TODO

  • saveBase64Images functionality could be moved into formatHtml. The current design was to give a clear separation from existing code, as well as give imageRepo access to the Page object.
  • This method relies heavily on the Drawio saveDrawing method. The Drawio class could be abstracted a bit. The directory and file naming in particular are very Drawio-centric.
  • To fully support this client-side, the Javascript restrictions on image blobs will need to be removed.

@ssddanbrown

Copy link
Copy Markdown
Member

Thanks for offering this PR @awarre, This solves a core current limitation in the API and will be especially useful to those converting & importing content from other formats. I'll go deeper through this soon to finish it off and get it implemented. Have assigned to next feature release but might sneak it into a sooner patch release instead.


Some notes primarily for myself to consider while I've been looking into something related to this:

  • From testing, Base64 image data in HTML seems to allow pretty much any whitespace (Including newlines) without breaking images.
  • We can use the normal core image saving methods instead of the drawio focused methods if we quickly decode beforehand, Will need to parse out the intended image type.

ssddanbrown added a commit that referenced this pull request Jun 2, 2021
- Added test cases to cover.
- Altered parsing logic to be a little less reliant on regex.
- Added new iamge repo method for creating from data.
- Added extension validation and additional type support.
- Done some cleanup of common operations within PageContent.
- Added message to API docs/method to mention image usage.

For #2700 and #2631.
@ssddanbrown
ssddanbrown merged commit 40ca50e into BookStackApp:master Jun 2, 2021
@ssddanbrown

Copy link
Copy Markdown
Member

Thanks again @awarre, Now all merged after some tweaks in 39928e1. Will be part of the next patch release.

I done a quick test using your original test case from your original issue, and it seemed to all work without any edits needed on the JavaScript side of things (The pixel avatar image was uploaded as expected after being pasted as a blob). This was using Firefox on Fedora 34.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants