Blocks are lost for new layouts if the template doesn't have the same region names the Default layout's template.

[docwilmot]

Description of the bug

New Layouts created with the Geary, or Rolph templates won't have a "Main content" block (when the Default layout is set to use the Moscone template).

New layouts created with a Flexible template won't have any blocks by default (when the Default layout is set to use the Moscone template).

This is because blocks are copied from the Default layout template into the new Layout being created. However, if the region names do not match across templates, some blocks will not be placed.

When switching from one Layout Template to another, we keep track of all "orphaned blocks" that do not have a matching region, set a "Refuge region", and then place all orphaned blocks into this region. We should do the same when creating a new Layout.

Steps To Reproduce

To reproduce the behavior:

1. Create a new layout choosing Geary or Rolph as the template, set the path to node/%.
2. No main content block appears (at least 1 block is lost)

For a more extreme example:

1. Create a flexible template
2. Create a new layout choosing the flexible template
3. no blocks at all appear (all blocks are lost)

Expected behavior

Blocks copied from the Default template be placed in the Refuge region, if there is not a region that matches in the selected template.

(The Refuge region is either 1 - the template's default region if provided, or 2 - a region named "content" if it exists, or 3 - the last region defined by the layout template.)

Additional information

Add any other information that could help, such as:

• Backdrop CMS version: 1.28

[stpaultim]

Just confirming that I am able to recreate the problem as well.

[argiepiano]

This is because, when you create a new layout, the layout is populated using the default layout blocks, which places the main page content in a region called content. All templates except Geary have a region called content. In Geary's case, the main regions are called third1, third2 and third3. Thus, the main page content block is never placed.

My suggestion is to rename third2 to content (and change the template file).

This has the potential to break all existing layouts that use that template, so an update hook will be needed to move all blocks in third2 to content. I'm wondering if this is really worth doing.... or we can just live with the fact that Geary is not automatically populated with main content.

[stpaultim]

@argiepiano I few other possible solutions (short or long term). I don't know if any or all of them make sense.

1) Add new content region in a way that makes sense and is non-disruptive for if it's left empty. 2) Modify the code that places blocks in layouts when creating a new layout, to recognize this specific problem with Geary and fix it. Then solve this problem in 2.x and remove this temporary solution. 3) Implement some help text or a warning message when creating a layout with Geary, telling people to add the content region manually.

[docwilmot]

Same problem applies with Rolph template actually. Both of those have multiple narrow columns in place of the full or 3/4 width columns other layouts use for content, which I think make those narrow columns impractical for a content region. I would suggest we add a full-width content just above the bottom region for both of those.

[argiepiano]

Add new content region in a way that makes sense and is non-disruptive for if it's left empty.

I would suggest we add a full-width content just above the bottom region for both of those

These two suggestions look like the best way to handle this,.

[klonos]

Seems like this .info property which is meant to handle this doesn't work as expected: https://github.com/backdrop/backdrop/blob/1.x/core/layouts/geary/geary.info#L16

; The default region automatically receives the "Main Content" block if needed.
default region = third2

https://github.com/backdrop/backdrop/blob/1.x/core/layouts/rolph/rolph.info#L17

; The default region automatically receives the "Main Content" block if needed.
default region = quarter1

I'd like to take a stab at this.

[klonos]

As it is currently, we are populating newly-created layouts with the blocks from the default layout. See layout_settings_form_save_submit():

  // If this is a new layout, populate blocks from default layout.

However we are only doing that if regions with matching names are found (which is the real culprit causing the bug reported here): https://github.com/backdrop/backdrop/blob/1.x/core/modules/layout/layout.admin.inc#L1112

      if (array_key_exists($region, $template_info['regions'])) {

I have fixed that on my local now, but before I file a PR for review, I need to ask the following:

1. If the default "Main content" block has been removed from the default layout, do we still want to create the new layout with a "Main content" block in its default region? (which is basically against the "if this is a new layout, populate it with blocks from the default layout." logic, because in that case we will be populating the new layout with a block that doesn't exist in the default layout) ...for the record, I'm fine either way - just asking what we need done so that I can adjust the logic accordingly.
2. With the current logic in place, even if the "Main content" block exists in the default layout, we are still skipping placing it in the new layout if there is no matching region. With the change that we will introduce here, we will be placing the "Main content" block even if no matching region was found - we will simply place it in whatever has been specified as 'default region' in the .info of the layout template (if any - see next point). So basically, a block that would have otherwise been "orphan" and not placed, is now placed in the default region. Should we follow this same logic with all the "orphan" blocks (no matching region in the new layout) and be placing them in the default region? ...or just special-case the "Main content" block? (again, I'm fine either way - just need to know).

3. There is this "edge" case:

   • create a flexible layout template
   • remove the default "content" row/region that comes with it, and save
   • try to create a layout with that flexible layout template

   Because there is no region with a matching name, the "Main content" block is not copied. Also, because there is no way to specify a 'default region' with flexible layout templates, we cannot apply the same logic that fixes this issue with the Geary and Rolph templates. The same issue will happen with any contrib or custom layout template that fails to specify a 'default region' in its .info file. Aside of the fact that we can consider making the 'default region' a requirement and provide a way for flexible templates to specify which region is their default (separate issues if we want that please), how do we want to fall back in these cases? ...just accept that these are edge cases (are they?) and the fact that we simply cannot address this bug in these scenarios?

[klonos]

• remove the default "content" row/region that comes with it, and save

No need for this step actually. The "Content" region in flexible layout templates is not content actually - it is a UUID instead. So this doesn't matter. All flexible templates have this problem and we cannot fix it unless we provide a way to specify which region is the default (separate issue, but only then we'll be able to fix this bug here for flexible templates).

[klonos]

If the default "Main content" block has been removed from the default layout, do we still want to create the new layout with a "Main content" block in its default region? ...

I was wrong to make that assumption - the "Main content" block cannot be removed from the default layout, so that is a non-issue.

[klonos]

OK, I decided to make certain assumptions and push what I have up for review/testing, so here it is: https://github.com/backdrop/backdrop/pull/4704

• The "Main content" block now gets added when using the Geary or Rolph templates.
• It even gets added if it happens to have been moved to another region in the default layout (so long as that other region has a region with a matching name in the new layout).
• If no matching region is found, then the block is placed in whatever region has been specified as the default region in the .info file.
• (bonus) Blocks are added from the default layout even when flexible templates are being used.

[stpaultim]

I did some testing. This Works For Me.

It now adds the Main Content block when I create layouts with Geary or Rolf.

[stpaultim]

I've added the Milestone Candidate tag. Does someone want to add this to the bugfix milestone?

[klonos]

It now adds the Main Content block when I create layouts with Geary or Rolf.

And also does the same when flexible templates are being used (no blocks were being added before). You can test and confirm that as well if you want.

Does someone want to add this to the bugfix milestone?

I do 😄 ...done!

[stpaultim]

There are comments on the PR, which is why this has been changed back to "Needs Work."

[herbdool]

Even though this is a "problem" I don't think it's a bug. Plus, the suggested changes are big enough that I'm more comfortable with this being in a minor release.

[docwilmot]

I'd consider this easily a bug though. The main content block just not showing up in two out of 10 core templates is a significant issue, that would certainly confuse new users.

[stpaultim]

I definitely consider the fact that the main content block is lost when switching layout templates to be a bug. However, the fact that this has not been reported before, suggests that it doesn't come up very often so it may not be that urgent and I'd not have any problem holding a solution for a minor release if core committers think that is a good idea.

I thin it would be good to solve this before (or at the same time) we commit any solution to: https://github.com/backdrop/backdrop-issues/issues/2894

I think that issue #2894 has the potential to increase the liklihood of folks encountering this problem.

[jenlampton]

I'd consider this easily a bug though. The main content block just not showing up in two out of 10 core templates is a significant issue, that would certainly confuse new users.

This sounds like a regression. We used to place content from missing regions into the last region (the refuge_region)? Did we loose that recently? Or was that only for switching layouts maybe, but not creating them? I'll go find the issue...

[jenlampton]

Okay, I found the original issue, but I think that was only for changing layout templates.

Every Layout Template should a "default region" that should display the main content block. If there is not one defined, but the template has a region named "content" - the "content" region will be the default region (but that's only intended to be a fallback) - maybe as a 3rd fallback we can use the same refuge_region as we do when changing layouts -- (but we should fix these other issues with core layouts before we start looking into that, and make sure they work without it.)

Is it possible that we forgot to add a default region for Geary or Rolph because all the other Radix layouts had a content region?

Nope, it's defined in Rolph:

; The default region automatically receives the "Main Content" block if needed.
default region = quarter1

And it's defined in Geary:

; The default region automatically receives the "Main Content" block if needed.
default region = third2

edit: I just made this comment on the PR, but I think it should go here too.

Default block still wont place for flexibles since the dont have a default region.

Can we assign a default region for flexible Layouts? That is the only thing that should determine where the default content block goes, so if we can add one we may not need these other changes?

It sounds like we might also have a problem with the default region not being used as intended (as shown with Geary and Rolph) but if we fix that, and add the missing piece to Flexible layout templates, that should be all we need? (ha!)

[stpaultim]

@jenlampton Did you see this comment from @klonos

https://github.com/backdrop/backdrop-issues/issues/6461#issuecomment-2071413645

[jenlampton]

@stpaultim no, but I just got there myself... playing catchup here :)

I think we've got 2 distinct problems : 1) the default region stopped working properly at some point, and 2) the Flexible templates don't have a default region. I don't know if either one of those should be split into a separate issue, but they are distinct issues and will have different solutions, probably both of which should have tests so seems like it?

And maybe a feature request:

• maybe as a 3rd option we can use the same refuge_region as we do when changing layouts, so it would go 1) if there is a default region, use this (and that will work for all core layouts) 2) if this missing, look for a region named "content", and then 3) If that is also missing, determine a refuge_region and use that as a last resort.

[jenlampton]

I just did a fresh install of Backdrop 1.x-dev and was able to create a new node/% layout in Geary, and the main page content block was placed correctly:

Can someone update the Steps to Reproduce?

Here's what I did:

• Fresh Backdrop install
• Switch Default layout template to Geary (confirm blocks placed correctly)
• Create a new Layout for node/%, leaving template as Geary (confirm blocks placed correctly)

I'll try again with Rolph.

[jenlampton]

Aha!

So I think the missing piece is that the Default layout needs to have the main content block in a different region than the new Layout. So either leave the Default layout alone, or choose a different template when creating the new Layout. I'm going to test the other way around to confirm.

[jenlampton]

When switching Templates to Rolph (for the default Layout), the main content block is still placed correctly into the new default region:

But when creating a new layout in Geary, the main content block is not placed at all:

It looks like whatever logic we are using for placing the main content block when switching templates is working, but that same calculation is not being used for placing the main content block when creating new Layouts. :/

[jenlampton]

This change is working for me for Geary and Rolph: https://github.com/backdrop/backdrop/pull/4777 All it does is use the same refuge_region handling from switching layouts. I'm going to test with flexible layouts.

[jenlampton]

I needed to make a little change, but now it's working for flexible templates also. I'm not super happy with this code, though. I'd prefer if we had one function that was used in both scenarios, so that we can't change one without the other in the future. I'm going to see if I can make that happen...

[jenlampton]

Okay, I'm more happy with https://github.com/backdrop/backdrop/pull/4777 now. It consolidates all the block placement code into layout.class.inc. Ready for testing in the sandbox.

maybe as a 3rd option we can use the same refuge_region as we do when changing layouts, so it would go 1) if there is a default region, use this (and that will work for all core layouts) 2) if this missing, look for a region named "content", and then 3) If that is also missing, determine a refuge_region and use that as a last resort.

I ended up doing something similar to this.

• I changed how we determine a refuge region to always 1) check for a default region, 2) check for a region named content, and then if both of those fail, 3) use the last region defined by the layout.
• I made both creating a new layout, and changing existing templates use this same refuge region

This ended up solving the problem for the missing default content block on Geary and Rolf, as well as ALL of the missing blocks on Flexible templates. I do think it would be nice if we could specify a default content region on flexi-templates, but that one can be the feature request :)

[stpaultim]

@jenlampton Your PR works for me!

[klonos]

... it would be nice if we could specify a default content region on flexi-templates, but that one can be the feature request :)

I agree 👍🏼 ...reviewing/testing the PR next 👀

[klonos]

Thanks @jenlampton 🙏🏼 ...I've reviewed the PR, and it's looking great! ...just left a couple of suggestions to consider.

I've also closed my PR in favor of yours.

[docwilmot]

I would suggest we add a full-width content just above the bottom region for both of those.

This may end up separate issue, but I still do think the columns in the Rolph and Geary templates are too narrow to really hold main content in most cases. Doesnt negate that we need this fix, however do we think we should also add a full-width row below those little columns? Mentioning here because may be wise to add that and make it the refuge region so users get a better experience from the start.

[klonos]

... do we think we should also add a full-width row below those little columns?

@docwilmot I think yes. Unlike the other core layout templates, these two (Rolph and Geary) do not have a content region, so we'd only be adding - not removing something. So unless doing that breaks existing sites, then I don't see why we can't do it. But separate issue though. ...the good thing about it is that once the change from this issue here is in, there'd be very little to do, since adding a content region to these two templates would cause them to automatically inherit the behaviour we are introducing/fixing here.

[docwilmot]

PR up to add content row region to Rolph and Geary https://github.com/backdrop/backdrop/pull/4808

[stpaultim]

@herbdool Do you still think this should be held for next minor release? (I'm looking for issues that a close to completion and still eligble to get into bugfix/1.29)

[izmeez]

The labels for this issue don't match the changes on Jun 6 and Jun 7.