Problem/Motivation

Trying to use #create_placeholder on a block causes blocks not to render at all in the Umami theme.

I did some debugging, and the #lazy_builder callback for those blocks gets called and works OK, but it seems like the block placeholder markup doesn't get into page markup at all, meaning there's nothing to replace. This is because Umami calls striptags in its page template, which when you have a bigpipe placeholder without any non-tag content, strips the entire thing.

Found in #3437499: Use placeholdering for more blocks.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3439017

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

catch created an issue. See original summary.

catch opened merge request !7354

catch’s picture

catch
he/him
Primary language English
commented

Theoretically we have test coverage for this, but I don't think it's testing the right thing:

BlockViewBuilderTest::testBlockViewBuilderBuildAlter()

This was added all the way back in the original issue #2543340: Convert BlockViewBuilder to use #lazy_builder (but don't yet let context-aware blocks be placeholdered).

catch’s picture

catch
he/him
Primary language English
commented
Title: Adding #create_placeholder to blocks is broken » Umami page.tpl.php breaks #create_placeholder for blocks
Component: render system » Umami demo

Got suspicious that only umami tests were failing, surely we had more block test coverage than this, and then looked at Umami's page.tpl.php and the bug is in there :(.

Postponing this on #3437499: Use placeholdering for more blocks since I'll include the fix in there, but leaving this open in case we want to split it out or remove more of this logic other than the striptags.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Did you mean core/profiles/demo_umami/themes/umami/templates/layout/page.html.twig?

It's not obvious to me what the problem is. I'd bet it's

{% if page.pre_header|render|striptags|trim is not empty or

which appears to be a work-around for #953034: [meta] Themes improperly check renderable arrays when determining visibility? 🫣

catch changed the visibility of the branch 3439017-create-placeholder to hidden.

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review

Extracted the commit from and put up an MR.

It's the strip tags that's the problem - placeholder HTML doesn't have any content outside the HTML tag, so Umami just discards it.

catch opened merge request !7896

catch’s picture

catch
he/him
Primary language English
commented
Title: Umami page.tpl.php breaks #create_placeholder for blocks » Umami page.tpl.php breaks block placeholders
Issue summary: View changes
smustgrave’s picture

smustgrave commented 
1) Drupal\FunctionalJavascriptTests\Ajax\AjaxTest::testAjaxFocus
    Failed asserting that two strings are equal.
    --- Expected
    +++ Actual

Seems unrelated to this change.

Question because I've similar code in the past. If you have debugging on then those conditions will now pass right? Due to the twig comments.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

To quote @catch from slack

There are issues around to try to figure out render array emptiness Vs not, umami shouldn't be doing template hacks for it.

So my concern in #10 is mute.

Will mark.

nod_ closed merge request !7896

  • nod_ committed 8144350a on 10.3.x 
    Issue #3439017 by catch, smustgrave: Umami page.tpl.php breaks block...

  • nod_ committed a989b1df on 10.4.x 
    Issue #3439017 by catch, smustgrave: Umami page.tpl.php breaks block...

  • nod_ committed a7a9469c on 11.0.x 
    Issue #3439017 by catch, smustgrave: Umami page.tpl.php breaks block...

  • nod_ committed 1fd79604 on 11.x 
    Issue #3439017 by catch, smustgrave: Umami page.tpl.php breaks block...
nod_’s picture

nod_
Primary language French
Location Lille
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 1fd796042d to 11.x and a7a9469cdc to 11.0.x and a989b1dfc9 to 10.4.x and 8144350adb to 10.3.x. Thanks!

Status: Fixed » Closed (fixed)

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