filter_autop returns self closing br element with slash, lets alter to br

+++ b/core/modules/filter/tests/src/Kernel/FilterKernelTest.php
@@ -987,7 +987,7 @@ public function testHtmlCorrectorFilter() {
     $this->assertEquals('<img src="http://example.com/test.jpg" />test', $f, 'HTML corrector -- Automatically close single tags.');
 
-    $f = Html::normalize('<br></br>');
+    $f = Html::normalize('<br>');
     $this->assertEquals('<br />', $f, "HTML corrector -- Transform empty tags to a single closed tag if the tag's content model is EMPTY.");
 

Shouldn't this test both until HtmlCorrector also puts out HTML 5 instead of changing from one to the other?

#28++

Rerolled patch against 11.x

The Needs Review Queue Bot → tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request → . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

On sites that are updated from Drupal 10.1 to Drupal 10.2 we see the following

On a site using Drupal Core 10.1 add a text format with both of these filters in the following order.

1. "Limit allowed HTML tags and correct faulty HTML"
2. "Convert line breaks into HTML (i.e.
and

)"

Then add a content type, and a text area field, and allow the text format to be used by your user etc.

Then edit source and add

<p>first line<br>
second line</p>

Note the line break character after the <br> (a UTF-8 0A character in the database).

Now update your site to Drupal 10.2 and see two <br> elements where there should be one.
In the database, this value for a text area will cause the issue of two <br> elements:

These have no line break character after the
.

These values will NOT exhibit the issue:

<p>first line<br>second line</p>
<p>first line<br />second line</p>

Note that these do not have a UTF-8 0A line break character after the
.

In a text format, moving the "Convert line breaks into HTML (i.e.
and

)" filter above the "Limit allowed HTML tags and correct faulty HTML" filter also resolves the issue of two
elements being output where only one was before.

I think I'm running into this issue, but the circumstances are (confusingly) different.

I'm on 10.2.1, and I'm currently upgrading from ckeditor4 contrib to ckeditor5. I'm not using "Convert line breaks into HTML (i.e. <br> and <p>)" before or after the upgrade, yet I'm seeing doubled linebreaks after the upgrade. If I open the offending node for editing, the linebreaks look normal (single), and saving the node fixes them.

The only two filters I have enabled (before and after the upgrade) are:

- Limit allowed HTML tags and correct faulty HTML
- Correct faulty and chopped off HTML

If I apply patch #30 with the upgrade, it seems to fix the doubled linebreaks. If I'm not using the "Convert line breaks..." filter, should this patch affect me?

Fixing title, hiding patches. Added some questions to the MR.

@longwave I tried and cannot reproduce this behavior on a clean install. My ckeditor upgrade involved ~10 commits, so I'm reviewing each one to see where the issue entered.

Well, that's embarrassing. I did indeed have the "convert line breaks" filter enabled. I've been trying to remove old text formats for some time, and I didn't realize one of the older formats was still being used.

So to update: patch #30 does fix the issue for me while upgrading from ckeditor4 to ckeditor5 on D10.2.1 while using the "Convert line breaks into HTML" filter.

I can confirm that the Merge Request as it is fixes this issue in Drupal 10.2.5. We just upgraded from CKEditor 4 to 5 and had extra linebreaks whenever the source contained a linebreak right inside the paragraph (not sure how our customers managed to get the linebreaks there).

This issue turns 10 years old in just a couple of months. As far as I can see, the MR title needs to be changed and one change to the test case needs to be reverted. I tried to do that, but Gitlab wouldn't let me.

trackleft2 → changed the visibility of the branch 2350049-filterautop-returns-self to hidden.

I've created a new fork to change the title of the merge request.
I've updated the test to account for @longwave's suggestion.
I've merged in upstream changes from 11.x and resolved merge conflicts.
I've hidden the original merge request branch.

Now I'm seeing these test failing.

---- Drupal\Tests\filter\Kernel\FilterKernelTest ----
Status    Group      Filename          Line Function                            
--------------------------------------------------------------------------------
Fail      Other      phpunit-462.xml      0 Drupal\Tests\filter\Kernel\FilterKe
    PHPUnit Test failed to complete; Error: PHPUnit 10.5.29 by Sebastian
    Bergmann and contributors.
    
    Runtime:       PHP 8.3.10
    Configuration: /builds/issue/drupal-2350049/core/phpunit.xml.dist
    
    ...F......F                                                       11 / 11
    (100%)
    
    Time: 00:07.706, Memory: 10.00 MB
    
    There were 2 failures:
    
    1) Drupal\Tests\filter\Kernel\FilterKernelTest::testLineBreakFilter
    'aaa
    bbb
    
    ccc': 'aaa
    bbb
    ccc' found. Filtered result: 'aaa
    bbb
    ccc
    '.
    Failed asserting that 'aaa\n
    bbb\n
    ccc\n
    ' [ASCII](length: 32) contains "aaa
    bbb
    ccc" [ASCII](length: 29).
    
    /builds/issue/drupal-2350049/core/modules/filter/tests/src/Kernel/FilterKernelTest.php:998
    /builds/issue/drupal-2350049/core/modules/filter/tests/src/Kernel/FilterKernelTest.php:492
    
    2) Drupal\Tests\filter\Kernel\FilterKernelTest::testHtmlCorrectorFilter
    HTML corrector -- Transform empty tags to a self-closed tag if the tag's
    content model is EMPTY.
    Failed asserting that two strings are equal.
    --- Expected
    +++ Actual
    @@ @@
    -''
    +''
    
    /builds/issue/drupal-2350049/core/modules/filter/tests/src/Kernel/FilterKernelTest.php:1123
    
    FAILURES!
    Tests: 11, Assertions: 206, Failures: 2.

Turns out when I was fixing the merge conflicts I blew away the fix.

Once the fix was back in place, one of the test failures went away, however I discovered that the second test was expecting <br> but was getting <br><br>.

The original version of the test was actually expecting <br><br> as well, so I changed it back, but changed the description.

https://git.drupalcode.org/project/drupal/-/merge_requests/9221/diffs#38...

Looks like now there are failed Functional Javascript tests, but I can't figure out what is going on. https://git.drupalcode.org/issue/drupal-2350049/-/jobs/2455600

I did notice some <br> tags in the test file which may be related, https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/ckedi...


There was 1 failure:
1) Drupal\Tests\ckeditor5\FunctionalJavascript\ImageTest::testAlignment
Failed asserting that a NULL is not empty.
/builds/issue/drupal-2350049/core/modules/ckeditor5/tests/src/FunctionalJavascript/ImageTestBase.php:500
/builds/issue/drupal-2350049/core/modules/ckeditor5/tests/src/FunctionalJavascript/ImageTestBase.php:478
FAILURES!
Tests: 10, Assertions: 377, Failures: 1, Skipped: 1.
---- Drupal\Tests\ckeditor5\FunctionalJavascript\ImageUrlTest ----
Status Group Filename Line Function
--------------------------------------------------------------------------------
Fail Other phpunit-5.xml 0 Drupal\Tests\ckeditor5\FunctionalJa
PHPUnit Test failed to complete; Error: PHPUnit 10.5.29 by Sebastian
Bergmann and contributors.

Runtime: PHP 8.3.10
Configuration: /builds/issue/drupal-2350049/core/phpunit.xml.dist

....F.... 9 / 9
(100%)

HTML output was generated.
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-1-65270370.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-2-65270370.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-3-65270370.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-4-65270370.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-5-65270370.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-6-65270370.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-7-65270370.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-8-65270370.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-9-65270370.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-10-65270370.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-11-65270370.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-12-65270370.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-13-65270370.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-14-65270370.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-15-65270370.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-16-82605180.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-17-82605180.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-18-82605180.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-19-82605180.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-20-82605180.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-21-82605180.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-22-82605180.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-23-82605180.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-24-82605180.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-25-82605180.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-26-39342599.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-27-39342599.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-28-39342599.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-29-39342599.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-30-39342599.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-31-39342599.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-32-39342599.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-33-39342599.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-34-39342599.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-35-39342599.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-36-39342599.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-37-39342599.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-38-39342599.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-39-39342599.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-40-47971540.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-41-47971540.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-42-47971540.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-43-47971540.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-44-47971540.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-45-47971540.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-46-47971540.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-47-47971540.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-48-47971540.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-49-47971540.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-50-67092286.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-51-67092286.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-52-67092286.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-53-67092286.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-54-67092286.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-55-67092286.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-56-67092286.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-57-44903977.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-58-44903977.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-59-44903977.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-60-44903977.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-61-44903977.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-62-44903977.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-63-44903977.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-64-44903977.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-65-76981562.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-66-76981562.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-67-76981562.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-68-76981562.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-69-76981562.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-70-76981562.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-71-76981562.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-72-34129822.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-73-34129822.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-74-34129822.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-75-34129822.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-76-34129822.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-77-34129822.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-78-34129822.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-79-34129822.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-80-32155407.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-81-32155407.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-82-32155407.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-83-32155407.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-84-32155407.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-85-32155407.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-86-32155407.html
http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_Tests_ckeditor5_FunctionalJavascript_ImageUrlTest-87-32155407.html
Time: 02:14.361, Memory: 10.00 MB
There was 1 failure:
1) Drupal\Tests\ckeditor5\FunctionalJavascript\ImageUrlTest::testAlignment
Failed asserting that a NULL is not empty.
/builds/issue/drupal-2350049/core/modules/ckeditor5/tests/src/FunctionalJavascript/ImageTestBase.php:500
/builds/issue/drupal-2350049/core/modules/ckeditor5/tests/src/FunctionalJavascript/ImageTestBase.php:478
FAILURES!
Tests: 9, Assertions: 389, Failures: 1.
---- Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLinkabilityTest ----

I updated the fork to include upstream changes from 11.x without any additional changes and now all the Functional Javascript test pass.

Issue summary should use the standard template so tagged for such.

smustgrave → changed the visibility of the branch 11.x to hidden.

Cleaning up MRs/patches

Ran test-only feature https://git.drupalcode.org/issue/drupal-2350049/-/jobs/2464098 which shows the coverage.

Not sure any BC concern, know filter module changes are difficult and take a while sometimes. But any 2 second research shows that
is correct for HTML5 so believe this change is correct.

Going to go on a limb, hope no BC concern, and mark

Everything looks in order here. The other MR, MR6262 has some questions/answers.

@trackleft2, thanks for commenting on the work you did and getting this to RTBC. I see that you create a new fork to change the title of the merge request. I appreciate that not everyone can edit all features of an MR but when a new MR is created it doesn't have the comments that are on the old one. That makes it just a bit harder for reviewers. It would help to use the existing MR and if you would like someone changed, ask in the issue or in Slack, #contribute. Thanks!

Can we resolve the questions on MR 6262 to confirm they're resolved in the new MR? thanks

Fine to self RTBC if they're resolved

smustgrave → closed merge request !6262

Believe the questions on MR 6262 have actually been addressed, went ahead and closed the MR too.

The question was:
— In here (and some later regexes) do we need to look for any of <br>, <br/>, <br />?
— I don't think there should be any more <br/> or <br /> now that this 🐛 Upgrade filter system to HTML5 Fixed was merged.

That's not reassuring, can we confirm that there is no way to have a <br /> in D11? what about when upgrading a CKE 4 config? does it hurt to keep the regexs for a couple of releases?