Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#13 | Screenshot 2014-12-24 08.01.36.png | 33.64 KB | larowlan |
#13 | Screenshot 2014-12-24 08.01.30.png | 74.07 KB | larowlan |
#13 | Screenshot 2014-12-24 08.01.05.png | 65.9 KB | larowlan |
#8 | block_content_in_place_editing-2284917-8.patch | 4.45 KB | Wim Leers |
Comments
Comment #1
catchDon't think this is critical.
Comment #2
olli CreditAttribution: olli commentedTried 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?
Comment #3
Wim LeersThis 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 theblock_content
) entity does too. ButBlockViewBuilder::buildBlock()
needs to "lift" (move) the#attributes
and#contextual_links
properties from the block contents (in this case of theblock_content
entity's render array) to the block itself. This works fine for all blocks except for theblock_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 toblock_content
entities. The#pre_render
pattern is applied to do less work in case of a render cache hit, butblock_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 ofblock_content
entities.Test coverage is still needed.
Comment #4
olli CreditAttribution: olli commentedWith #3, I can now edit and quick edit custom blocks!
No, they can be rendered with views for example.
Comment #5
Wim Leers#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? :)
Comment #6
alexpottWe should have a comment as to why this is acceptable.
Comment #7
Wim LeersA duplicate of this was reported at #2350741: Quick Edit not displayed in contextual links after Minimal profile install.
Comment #8
Wim LeersDone.
Tests added.
Comment #9
Kyna CreditAttribution: Kyna commentedHello, 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.
Comment #10
alexpott@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.
Comment #11
Wim LeersKyna: 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! :)
Comment #12
Wim LeersAnother duplicate of this was reported at #2375849: Quick Edit missing .
Comment #13
larowlanWorks, and adds tests - great job
Screenshots from manual testing
Comment #14
alexpottThis 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!