Problem/Motivation

I just noticed in-place editing of blocks is broken *again*.

I already fixed it in #2171269: Handle block rendering's #attributes correctly (fixes in-place editing of custom blocks) and added test coverage back then, but only at the block level, not the custom_block (now block_content) entity. And it seems that something is broken in there.

Proposed resolution

Fix it.

Remaining tasks

TBD

User interface changes

None.

API changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

Priority: Critical » Major

Don't think this is critical.

olli’s picture

Status: Active » Needs review
FileSize
1.87 KB

Tried to figure out the problem and looks like since #2099131: Use #pre_render pattern for entity render caching the contextual links (edit and quick edit) for custom blocks are added too late. BlockContentViewBuilder::alterBuild() and quickedit_entity_view_alter() are invoked after BlockViewBuilder::buildBlock(). Here's a patch that adds the needed #contextual_links and #attributes before they are processed in buildBlock(). Any other ideas?

Wim Leers’s picture

Title: In-place editing of custom blocks broken *again* (its custom attribute is lost during block rendering) » In-place editing of custom blocks broken *again* (because attributes and contextual links of custom blocks are lost)
Assigned: Wim Leers » Unassigned
FileSize
2.69 KB
+++ b/core/modules/quickedit/quickedit.module
@@ -165,9 +165,9 @@ function quickedit_preprocess_field(&$variables) {
-function quickedit_entity_view_alter(&$build, EntityInterface $entity, EntityViewDisplayInterface $display) {
+function quickedit_entity_build_defaults_alter(&$build, EntityInterface $entity) {

This is less than ideal, because this hook is invoked always, even on render cache hits, because it is was IIRC added specifically to allow modules to alter the cache ID of render cached entities.

Finally, this hook is not invoked for all entities, only for those that extend the default EntityViewBuilder.


The key problem is that blocks use the #pre_render pattern, but entities (and hence the block_content) entity does too. But BlockViewBuilder::buildBlock() needs to "lift" (move) the #attributes and #contextual_links properties from the block contents (in this case of the block_content entity's render array) to the block itself. This works fine for all blocks except for the block_content blocks, because they are entities that themselves are rendered lazily (using the #pre_render pattern)… just because they are entities.
The solution is therefore simple: don't apply the #pre_render pattern to block_content entities. The #pre_render pattern is applied to do less work in case of a render cache hit, but block_content entities are always rendered as blocks and blocks already have their own render caching.

This patch therefore applies the #pre_render callback immediately, and disables render caching of block_content entities.

Test coverage is still needed.

olli’s picture

With #3, I can now edit and quick edit custom blocks!

block_content entities are always rendered as blocks

No, they can be rendered with views for example.

Wim Leers’s picture

#4: Ok, that's true, but that's an insane edge case. The 99% use case is that these content_block entities will be rendered in blocks, and in that case, render caching is completely unnecessary. Therefore I think this change is fine; it doesn't break the Views use case that you mention.

Any chance you want to write test coverage? :)

alexpott’s picture

+++ b/core/modules/block_content/src/Entity/BlockContent.php
@@ -53,6 +53,7 @@
+ *   render_cache = FALSE,

We should have a comment as to why this is acceptable.

Wim Leers’s picture

Wim Leers’s picture

Issue tags: -sprint, -Needs tests
FileSize
4.45 KB
2.34 KB

We should have a comment as to why this is acceptable.

Done.

Tests added.

Kyna’s picture

Hello, I have try and the last patch fix the problem, "Quick edit" work be fine now, but I have an another problem with custom view, you can see the problem on this page after login (login form inside footer) :

-

If I hover the header or footer, the icone "Edit view" is displayed :
http://i.imgur.com/JddQTek.png

It's a bug, on Bartik theme, this icon is displayed only when I hover the content and not when I hover the header or footer.

alexpott’s picture

@Kyna you should not add user login details - these could be used maliciously. Also please don't add additional bugs to an issue. Maybe this bug has been filed for bartik already - or you should create a new issue.

Wim Leers’s picture

Kyna: that's probably a bug in your custom theme. But since that's a separate problem, please create a new issue. And like Alex says, you shouldn't post user login details publicly. That's why I asked you to send them to me via my d.o contact form! :)

Wim Leers’s picture

Another duplicate of this was reported at #2375849: Quick Edit missing .

larowlan’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
65.9 KB
74.07 KB
33.64 KB

Works, and adds tests - great job

Screenshots from manual testing

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes.. Committed 54c74fd and pushed to 8.0.x. Thanks!

  • alexpott committed 54c74fd on 8.0.x
    Issue #2284917 by Wim Leers, larowlan, olli: In-place editing of custom...

Status: Fixed » Closed (fixed)

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