GrapesJS / grapesjs

Free and Open source Web Builder Framework. Next generation tool for building templates without coding
https://grapesjs.com
BSD 3-Clause "New" or "Revised" License
22.36k stars 4.05k forks source link

BUG: Pasting a component in root body layer throws TypeError #4000

Closed zachsnoek closed 2 years ago

zachsnoek commented 2 years ago

GrapesJS version

What browser are you using?

Chrome v89

Reproducible demo link

https://grapesjs.com/demo.html

Describe the bug

How to reproduce the bug?

  1. In the official demo, navigate to the layer manager and copy any layer with cmd+c
  2. Click the root "Body" layer and paste with cmd+p
  3. Navigate to the browser console and see the error

What is the expected behavior? Pasting in the body layer probably shouldn't be allowed, but it shouldn't throw an error.

What is the current behavior? Pasting in the root Body layer throws this exception:

Uncaught TypeError: Cannot read property 'indexOf' of undefined
    at PasteComponent.js:13
    at Array.forEach (<anonymous>)
    at run (PasteComponent.js:10)
    at r.callRun (CommandAbstract.js:90)
    at Object.runCommand (index.js:388)
    at Object.method (index.js:158)
    at w (keymaster.js:143)
    at HTMLDocument.<anonymous> (keymaster.js:330)

The error is coming from the paste commend; comp.collection is undefined.

I don't know this codebase well enough to know if .collection on this component should return children. We were able to fix this temporarily by overriding the paste command and adding the following guard:

// ...
const coll = comp.collection;
if (!coll) {
    return;
}
const at = coll.indexOf(comp) + 1;
// ...

If this is a fine solution, let me know and I'll happily open a PR.

Code of Conduct

artf commented 2 years ago

Thanks @zachsnoek yeah, as the wrapper (body) is a root component it doesn't have the related collection. Probably we might need to update the paste logic but your current fix is a good patch for now to avoid such an error so, the PR is highly welcome 👍

zachsnoek commented 2 years ago

Thanks @zachsnoek yeah, as the wrapper (body) is a root component it doesn't have the related collection. Probably we might need to update the paste logic but your current fix is a good patch for now to avoid such an error so, the PR is highly welcome 👍

Sounds good; I'll put up a PR by the end of the week :+1:

GuiMoraesDev commented 2 years ago

Guys, I'm facing a similar problem To me, the error happens on FileUploader

Screenshot from 2021-12-16 17-10-32

I've tried to enter on grapes demo to see if this behavior happens there too, and it's happening!

I realize that If I clean my LocalStorage and reload the page, then the Editor loads the standard template, and I can drop any block inside it that will work well, but if I clean the editor and try to drop a Text block on Editor, the Editor saves an image tag in HTML key Take a look bellow

image

Any Idea that can help to solve this problem?

artf commented 2 years ago

Fixed here https://github.com/artf/grapesjs/pull/4008