Skip to content
This repository has been archived by the owner on Jan 5, 2018. It is now read-only.

Issue #2712111 by slashrsm, paranojik: Outdated cached version of embedded entity #220

Conversation

paranojik
Copy link

Problem was that the data attributes were added to the embedded entity itself. This caused the same data attributes to appear everywhere the same view mode was used/displayed on the site. That also meant that the cached version was not invalidated until the embedded entity's cache was cleared.

To prevent all mentioned problems we wrap the embedded entity and append the data attributes to the wrapper element.

…edded entity is displayed when embed tag is updated.
@davereid
Copy link
Member

It would help me to understand what's actually wrong if we had a test case that failed first to prove this is fixing something.

@davereid
Copy link
Member

Would this also be related to #187 which is the fix for the bubbleable metadata?

@slashrsm
Copy link
Member

Would this also be related to #187 which is the fix for the bubbleable metadata?

I don't think this is related.

It would help me to understand what's actually wrong if we had a test case that failed first to prove this is fixing something.

Check issue summary in https://www.drupal.org/node/2712111. We found another problem while working on the patch. Will describe it in the issue summary now.

@wimleers
Copy link

wimleers commented May 4, 2016

It would help me to understand what's actually wrong if we had a test case that failed first to prove this is fixing something.

+100

@wimleers
Copy link

wimleers commented May 4, 2016

The filter that renders embedded entities must associate the necessary cacheability metadata. It sounds like that's not happening.

This is therefore basically a duplicate of https://www.drupal.org/node/2593379, which has PR #187. This only covers one of the many potential side-effects/consequences. The lack of test coverage for this is what's the real problem: EntityEmbedFilterTest only tests if something works at all, not if it is updated when necessary. We should at least test:

  • that if an embedded entity is modified, the host entity's rendered output is also updated
  • that if a tag on an embedded entity is added or removed, the host entity's rendered output is also updated
  • that an embedded entity that becomes unpublished, it is also no longer rendered by the host entity's rendered output, because it's no longer accessible … but only for anonymous users

@wimleers
Copy link

wimleers commented May 4, 2016

(Cross-posted from https://www.drupal.org/node/2712111#comment-11155929. These split conversations are mind-bending BTW.)

@slashrsm
Copy link
Member

slashrsm commented May 5, 2016

This is therefore basically a duplicate of https://www.drupal.org/node/2593379, which has PR #187.

I am not sure about this. https://www.drupal.org/node/2593379 is about metadata from the embedded entity not being bubbled up. This is about entity_embed specific attributes being added to the part of the render array that belongs to the embedded entity and them being cached as part of it.

In order to reproduce this embed a node using teaser view mode and use align/caption. Then navigate to the front page and inspect markup of the same entity when appearing on it's own (not embedded). You will see embed-specific attributes being added to it (data-align, data-caption), which clearly do not belong there.

Will work on test that will prove that, but it might take me few days.

@davereid
Copy link
Member

https://www.drupal.org/node/2712111 has been fixed.

@davereid davereid closed this Mar 29, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants