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

[Bug]: Default image placeholder's src attribute generated / exported as <svg> element #2619

Closed
mcottret opened this issue Mar 3, 2020 · 2 comments

Comments

@mcottret
Copy link

mcottret commented Mar 3, 2020

Hi, and thanks for the great library !

We might have found the following bug when playing around with it:

When adding a new default "Image Block" without further configuration (leaving the default image placeholder), the exported / generated HTML code embeds the default svg element as the image's src attribute value (cf attached screenshots).

Screenshot 2020-03-03 at 4 31 40 PM
Screenshot 2020-03-03 at 4 31 06 PM

After digging a bit into the code, it appears that the grapesjs/src/dom_components/model/ComponentImage.js's getAttrToHTML function should call this.getSrcResult (not sure whether or not to use the fallback option) rather than this.get('src') to retrieve the base64 encoded image src attribute (cf https://github.com/artf/grapesjs/blob/dev/src/dom_components/model/ComponentImage.js#L78) .

Note: this would not satisfy a case where the embedAsBase64 option would be set to false, maybe in this case a public image placeholder URL could be used (TBD) ?

I'd be happy to open a PR to fix it, let me know :)

@mcottret mcottret changed the title [Bug]: Default image placeholder src generated / exported as <svg> element [Bug]: Default image placeholder's src attribute generated / exported as <svg> element Mar 3, 2020
@mcottret mcottret changed the title [Bug]: Default image placeholder's src attribute generated / exported as <svg> element [Bug]: Default image placeholder's src attribute generated / exported as <svg> element Mar 3, 2020
@artf
Copy link
Member

artf commented Mar 3, 2020

Thanks for the report Mathieu

grapesjs/src/dom_components/model/ComponentImage.js's getAttrToHTML function should call this.getSrcResult (not sure whether or not to use the fallback option) rather than this.get('src') to retrieve the base64 encoded image src attribute

Correct, ignore the fallback option, it's used in the canvas only when the image is failed to load.

Note: this would not satisfy a case where the embedAsBase64 option would be set to false, maybe in this case a public image placeholder URL could be used (TBD) ?

embedAsBase64 option is for the file upload, this has nothing to do with this issue

So... go ahead for the PR! 👍

@mcottret
Copy link
Author

mcottret commented Mar 4, 2020

Thanks ! #2620 opened.

@artf artf closed this as completed in ba27279 Mar 5, 2020
artf added a commit that referenced this issue Mar 5, 2020
…der-src

Fix ComponentImage's default src not being base64 encoded. Fixes #2619
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants