Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Toolkit\File] Option to disable base64 encoding in `dataUri()` method #2276

Closed
tillprochaska opened this issue Nov 4, 2019 · 2 comments
Closed

Comments

@tillprochaska
Copy link

@tillprochaska tillprochaska commented Nov 4, 2019

The dataUri method is quite handy, especially for inlining small resources like icons etc. The dataUri method always uses Base64 encoding, even if the file’s format is a plaintext format (e. g. SVG).

In those cases, encoding the file using Base64 wouldn’t be necessary. That’s not a severe problem, but it’s inefficient. Encoding using Base64 adds an overhead of ~33 percent in file size. Additionally, Base64-encoded data might reduce the efficiency of gzip compression.

I’d propose to disable Base64 encoding for plaintext formats or, alterantively, add an option to the dataUri method to disable it manually. As there are some edge cases where it might be required or otherwise sensible to encode plaintext data, I think the latter would be best.

@lukasbestle lukasbestle transferred this issue from getkirby/ideas Nov 4, 2019
@lukasbestle lukasbestle added this to the 3.3.1 milestone Nov 4, 2019
@lukasbestle

This comment has been minimized.

Copy link
Contributor

@lukasbestle lukasbestle commented Nov 4, 2019

I like the idea of a new param like $file->dataUri(bool $base64 = true) that makes it easy to control the behavior. The default would be backwards-compatible as well.

@lukasbestle lukasbestle changed the title [Toolkit\File] Don’t use Base64 in `dataUri` if file is plain text [Toolkit\File] Option to disable base64 encoding in `dataUri()` method Nov 4, 2019
@bastianallgeier bastianallgeier modified the milestones: 3.3.1, 3.3.2 Nov 19, 2019
@distantnative distantnative self-assigned this Nov 23, 2019
distantnative added a commit that referenced this issue Nov 23, 2019
distantnative added a commit that referenced this issue Nov 23, 2019
lukasbestle added a commit that referenced this issue Nov 24, 2019
distantnative added a commit that referenced this issue Nov 24, 2019
@distantnative

This comment has been minimized.

Copy link
Contributor

@distantnative distantnative commented Nov 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.