Quick Edit of host entity "leaks" into embedded entities: disable Quick Edit

I'm seeing this issue as well. See attached images for screenshots of the issue.

This is still an active issue.

Reproduced.

This actually looks like a bug in Quick Edit in Drupal core. It indeed only affects title fields of embedded entities. I vaguely recall that Quick Edit had to special case those … but obviously got something wrong in the logic! Will need to do some JS debugging to figure this out.

Also, FYI: #3011228: Missing `data-quickedit-entity-id` attribute prevents in-place editing of embedded entities's patch solves that issue, but does not help fix this.

Updating the title to explicitly communicate that this is indeed a core bug.

I cannot reproduce this when embedding media entities, which is the most common use case for entity_embed.

Keeping this marked as normal bug report, but removing it from #2577891: Entity Embed 8.x-1.0.0-rc1 release.

I suspect I may have been testing #2940029: Add an input filter to display embedded Media entities when I wrote #8.

Quoting #2940029-38: Add an input filter to display embedded Media entities:

Next up is test coverage, as described in #27, specifically these two bullets:

• […]
• in-place editing and contextual links are explicitly disabled; the reason being that it's going to be a terrible UX if content authors can in-place edit embedded entities, because most of them will assume that will only affect that specific embed (to be ported to Entity Embed, and this will also affect its test coverage)

Entity Embed definitely is still susceptible to this, as this failing test shows.

The draft core patch already addressed this. Let's port that logic, but let's first prove it's broken right now.

This is ported as close as possible from #2940029: Add an input filter to display embedded Media entities's patch. I think this will cause some test failures though because it is running for all @EntityEmbedDisplay plugins right now, rather than only the view mode-based one. I'll fix that tomorrow :)

Assigning to me; I'll continue tomorrow.

Reverted changes that #11 made to EntityEmbedBuilder, because like #11 predicted, it caused problems elsewhere.

Also, this took me multiple hours to figure out. @EntityEmbedDisplay plugins building on top of formatters in core and being derived and being configurable made this super complex — at least for me.

Green again — yay :)

Confusingly, #9 passed tests. It should not have. And actually … it did not. This is DrupalCI's reporting being flaky.

At https://www.drupal.org/pift-ci-job/1329534 you can see the last daily test run against the HEAD of the branch. 88 passes. #9 is adding a new test, and yet still only 88 passes. This means the new test was not executed.

And sure enough:

00:04:51.266 Tests to be run:
00:04:51.267   - Drupal\Tests\entity_embed\Functional\ViewModeFieldFormatterTest
00:04:51.267   - Drupal\Tests\entity_embed\Functional\RecursionProtectionTest
00:04:51.267   - Drupal\Tests\entity_embed\Functional\ImageFieldFormatterTest
00:04:51.268   - Drupal\Tests\entity_embed\Functional\FileFieldFormatterTest
00:04:51.268   - Drupal\Tests\entity_embed\Functional\EntityReferenceFieldFormatterTest
00:04:51.268   - Drupal\Tests\entity_embed\Functional\EntityEmbedUpdateHookTest
00:04:51.269   - Drupal\Tests\entity_embed\Functional\EntityEmbedTwigTest
00:04:51.269   - Drupal\Tests\entity_embed\Functional\EntityEmbedHooksTest
00:04:51.270   - Drupal\Tests\entity_embed\Functional\EntityEmbedEntityBrowserTest
00:04:51.270   - Drupal\Tests\entity_embed\Functional\EntityEmbedDisplayManagerTest
00:04:51.270   - Drupal\Tests\entity_embed\Functional\EntityEmbedDialogTest
00:04:51.271   - Drupal\Tests\entity_embed\Kernel\EntityEmbedFilterTranslationTest
00:04:51.271   - Drupal\Tests\entity_embed\Kernel\EntityEmbedFilterTest
00:04:51.271   - Drupal\Tests\entity_embed\Kernel\EntityEmbedFilterOverridesTest
00:04:51.272   - Drupal\Tests\entity_embed\Kernel\EntityEmbedFilterLegacyTest
00:04:51.272 
00:04:51.272 Test run started:

Note the absence of \Drupal\Tests\entity_embed\Kernel\EntityEmbedFilterDisabledIntegrationsTest. Same in #14.

Looks like the class name doesn't match the filename, that probably caused it.

I'm not sure about this:

+    if ($this->getPluginId() !== 'entity_reference:entity_reference_entity_view') {
+      return $build;
+    }

Isn't it possible that there are other plugins that render the entity that don't have that plugin id? Should we allow other plugins to opt to what comes after the return statement here? Could we check if it inherits from the base class instead?

For the alt and title we added another attribute, $definition['supports_image_alt_and_title']), perhaps we should follow a similar approach here?

$definition['renders_entity']) or something like that.

Ah, 91 tests passed — this time it worked! :)

Now let's fix the sole coding standards violation and upload a fail/pass patch.

Isn't it possible that there are other plugins that render the entity that don't have that plugin id? Should we allow other plugins to opt to what comes after the return statement here? Could we check if it inherits from the base class instead?

We want to do this whenever a view mode/EntityViewDisplay-powered representation of an entity gets rendered, because that's where contextual links and Quick Edit may appear.

That's an important difference with supports_image_alt_and_title, where for example alt may be overridden both when formatting an image Media entity's image directly and when viewing an image Media item in whichever view mode.

However … it's possible that contrib modules are subclassing the existing formatter to layer on special behaviors. Turns out for example entity_reference_override does this!

Fortunately there's a reliable way to detect whether a render array is generated by EntityViewBuilder, which is really what we're after.

I think #17 is ready :)

Also: there's nothing upstream-related nor JavaScript-related here anymore, so updating title and tags.

Crediting everybody who participated in this issue 🙏

🚢