Follow-up to #2545972: Remove all code usages SafeMarkup::checkPlain() and rely more on Twig autoescaping

Problem/Motivation

The DiffFormatter is an HTML display of a diff - it needs to be able to display file diffs in a browser by escaping the markup the text contains. It also adds markup to denote which parts of each line are different - this should not be escaped.

Proposed resolution

Remove usage of SafeMarkup::checkPlain() in DiffFormatter and make it responsible for all escaping.

Remaining tasks

  • Review
  • Commit

User interface changes

None

API changes

The behaviour of HWLDFWordAccumulator is slightly changed to no longer escape markup - but it never should have been doing that.

Data model changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Status: Active » Needs review
FileSize
7.16 KB

The patch adds assertion to the config diff test that exercise every line changed here.

star-szr’s picture

Status: Needs review » Reviewed & tested by the community

Looks great, I like the added test coverage. RTBC if green :)

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2560055.2.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
FileSize
2.59 KB
7.23 KB

Made the assertions even better and fixed an incorrect one.

stefan.r’s picture

Status: Needs review » Reviewed & tested by the community

Back to RTBC as this should be green now that we do

+++ b/core/modules/config/src/Tests/ConfigImportUITest.php
@@ -332,10 +332,10 @@ function testImportDiff() {
-    $this->assertText(Html::escape('<p>foobar</p>'));
...
+    $this->assertText(Html::escape("404: '<em>herp</em>'"));
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed 4ba004d on 8.0.x
    Issue #2560055 by alexpott: Remove all usages SafeMarkup::checkPlain()...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.