Problem/Motivation

I discovered recently that the crucial filter format migration for D7 is flawed in its handling of the format ID. It performs various transformations on the source ID that were necessary when migrating formats from D6 (which had numeric format IDs), but are wholly unnecessary when migrating from D7, which identifies formats by machine name.

In a worst-case scenario, this flaw breaks migration of nodes, text field values, and other things that depend on filter formats. It is therefore Migrate-critical and blocks the D7 migration paths for nodes, comments, and users.

Proposed Resolution

The d7_filter_format migration should migrate the format ID verbatim, without any processing.

Remaining Tasks

Review and commit the patch.

API/UI Changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

phenaproxima created an issue. See original summary.

mikeryan’s picture

Status: Needs review » Reviewed & tested by the community

It's nice when the right way is the simple way.

webchick’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

If this breaks so many things, we really should correct our tests so they demonstrate it.

phenaproxima’s picture

Here ya go! A fail patch to prove the problem, with the failing assert merged into the passing patch.

phenaproxima’s picture

Issue tags: -Needs tests

The last submitted patch, 4: 2555089-4-FAIL.patch, failed testing.

mikeryan’s picture

Status: Needs review » Needs work
+++ b/core/modules/filter/src/Tests/Migrate/d7/MigrateFilterFormatTest.php
@@ -57,7 +58,11 @@ public function testFilterFormat() {
+    $this->assertEntity('plain_text', 'Plain text', [ 'filter_html_escape', 'filter_url', 'filter_autop']);

Got a stray space in there. RTBC with that removed.

phenaproxima’s picture

Status: Needs work » Needs review
FileSize
2.12 KB

Fixed.

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

RTBCing as per #7.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Awesome sauce!

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 8fbc6da on 8.0.x
    Issue #2555089 by phenaproxima, mikeryan: d7_filter_format migration...

Status: Fixed » Closed (fixed)

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

Wim Leers’s picture

FYI: this is still super confusing for sites that upgraded from D5 or D6 to D7; they do still have numerical filter format IDs: #3095337: Text formats that are migrated from D6 to D7 to D8 get a numerical ID instead of a sensible name.