[DrupalHtmlEngine] Follow-up for script and style tags in CKEditor 5

Created on 1 April 2024, 3 months ago
Updated 2 April 2024, 3 months ago

Problem/Motivation

From 🐛 JavaScript operators in Needs work

I do not get why we have this test case:

      'unescaped script tag' => [
        <<<HTML
        <script>
          const example = 'Consider this string: <!-- <script>';
          console.log(example);
        </script>
        <!-- despite appearances, this is actually part of the script still! -->
        <script>
          let a = 1 + 2; // this is the same script block still...
        </script>
        HTML,

This is not the test I added. Can someone explain to me despite appearances, this is actually part of the script still! because I do not see a reason why this should be only one script tag. For me it's 2 different script tags and should be treated as such. If it's to get around the behavior I'm seeing below it should not work that way.

I added the test case:

<script>
const example = 'Consider this string: <!-- <script>';
console.log(example);
</script>

And that is transformed into:

<script>
const example = 'Consider this string: &amp;lt;!-- &amp;lt;script&amp;gt;';
console.log(example);
&amp;lt;/script&amp;gt;
&amp;lt;/body&amp;gt;</script>

Above creates a syntax error when it's viewed on the node. The actual result should be somethings like:

<script>
const example = 'Consider this string: \x3C!-- \x3Cscript>';
console.log(example);
</script>

So while there is not an issue with our code, CKEditor upstream doesn't parse this correctly and adds a closing script and body tag too early. Having a second script tags seems to go around the problem but fact is that there is still an issue and at least a follow-up and/or upstream issue is needed to fully support <script> tags in wysiwyg.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

🐛 Bug report
Status

Active

Version

11.0 🔥

Component
CKEditor 5 

Last updated about 4 hours ago

Created by

🇫🇷France nod_ Lille

Live updates comments and jobs are added and updated live.
  • JavaScript

    Affects the content, performance, or handling of Javascript.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @nod_
  • 🇫🇷France nod_ Lille
  • 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
  • First commit to issue fork.
  • I do not get why we have this test case:

    I believe this might be a non-issue, as I included the test case directly from the HTML Standard page mainly to test that CKEditor or our HtmlBuilder doesn't mangle up the existing markup if that's what the original source code is.

    I believe browser quirks like these should be left for the content editor to handle themselves rather than us converting it for them, especially if it might involve changing string values like:

    const example = 'Consider this string: <!-- <script>';
    

    to:

    const example = 'Consider this string: \x3C!-- \x3Cscript>';
    

    It can lead to unintentional bugs.

    And with regards to:

    And that is transformed into:

    <script>
    const example = 'Consider this string: &amp;lt;!-- &amp;lt;script&amp;gt;';
    console.log(example);
    &amp;lt;/script&amp;gt;
    &amp;lt;/body&amp;gt;</script>
    

    Is it actually being double encoded to HTML in your tests, or is the D.O formatter in the issue encoding it for you? As my local tests doesn't involve any encoded HTML entities as that's what 🐛 JavaScript operators in Needs work was meant to mitigate in the first place.

    Nonetheless, I've added an updated test-case to showcase the behaviour that'll also work for test-only pipelines even though we are no longer using a @dataProvider.

  • Pipeline finished with Failed
    3 months ago
    Total: 1537s
    #135499
Production build 0.69.0 2024