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: The editor does not remove the dead script blocks #3316

Closed am1rb closed 3 years ago

am1rb commented 3 years ago

Version:

0.16.34

Are you able to reproduce the bug from the demo?

[ ] Yes [x ] No

As I understand the code import dialog skips all the HTML scripts, so I can not reproduce the issue on the demo

What is the expected behavior?

The editor must remove dead script blocks before appending a new script block at the end of the generated HTML

Describe the bug detailed

I defined a component with script (https://grapesjs.com/docs/modules/Components-js.html) in my project. When I add this component into the editor and save the template, the editor generates the HTML and appends a script block at the end of the body to initiate the related components. The output HTML is something like this:

<head>
    <!-- Generated HTML -->
    <script>/* The appended script block to initiate my custom components */</script>
</head>

If I load the above output on another editor instance and save the template again it appends another script block and does not remove the old ones and the output will be something like this:

<head>
    <!-- Generated HTML -->
    <script>/* The appended script block to initiate my custom block */</script>
    <script>/* A new script block to initiate my custom block */</script>
</head>

And if I repeat this behavior, It appends a new script block to my template, and it leads to having a lot of dead codes on my final template.

<head>
    <!-- Generated HTML -->
    <script>/* Dead Script 1 */</script>
    <script>/* Dead Script 2 */</script>
    <script>/* .... */</script>
    <script>/* A new script block to initiate my custom block */</script>
</head>
Andrew-Chen-Wang commented 3 years ago

Typically inline scripts go in the body at the bottom. Try putting it there maybe?

am1rb commented 3 years ago

I do not append anything by myself. The editor appends the scripts related to my components in a new script tag at the end of the generated HTML. Please take a look at this file: https://github.com/artf/grapesjs/blob/dev/src/editor/model/Editor.js#L513 I think maybe there is something wrong here. The editor appends a new script block without removing the old one.

Ju99ernaut commented 3 years ago

I'm unable to reproduce this, maybe you can provide more information on the custom component itself, anyway I suspect this is a storage related issue. Are you storing then loading pages from the generated html?

artf commented 3 years ago

I think maybe there is something wrong here. The editor appends a new script block without removing the old one.

The editor will output scripts if the wrapper contains at least 1 component containing script property, so, there is no need to remove stuff as it is solely related to your components.

From what I can see by reading your issue If I load the above output on another editor instance, this is completely wrong, you have to load the JSON and not the HTML. Read carefully this guide https://grapesjs.com/docs/modules/Storage.html

I guess this is just a wrong Storage configuration, so I'm closing this issue.

Ashwinvalento commented 9 months ago

The issue can be reproduced if we set allowScripts: true . I have created a reproducable demo here https://codepen.io/ashwinvalento/pen/jOJMPqx

Steps to Reproduce:

  1. Drop the "Duplicate Script Block" to the canvas
  2. Click on the Import code option. Notice that there is only 1 script block of the "Duplicate Script Block"
  3. Click import button.
  4. Again click on the Import code option and notice that there are 2 blocks now.

@artf @Ju99ernaut