SystemMainBlock::build does not always return an array.

But documentation clearly says it should:
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Block%21B...

It can produce errors like this:
https://www.drupal.org/node/2812721

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jiv_e created an issue. See original summary.

jiv_e’s picture

jiv_e’s picture

Status: Active » Needs review
FileSize
514 bytes
jiv_e’s picture

Dinesh18’s picture

I have verified the API and the function doesn't return array. I manually reviewed the patch and it looks like the solution provided by you is perfect.
+1 to RTBC. Let's wait for some other to review the patch as well.

BR0kEN’s picture

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

borisson_’s picture

Status: Needs review » Closed (duplicate)

Actually - reading the other issue - it seems like this was already fixed there. I think that means we can close this issue. The original proposed fix in that issue was the same as the one proposed here.

jiv_e’s picture

Status: Closed (duplicate) » Needs work

@borisson_ Thanks for taking this up! I quess you refer to this issue: https://www.drupal.org/node/2819219

The original fix was the same, but @xjm reverted it. See https://www.drupal.org/node/2819219#comment-11767286.

So this is still not fixed - reopening.

As one downside of the current solution is that we have to mention in our documentation that it's possible SystemMainBlock::build to return something else than an array. See more detailed argumentation here. So either we fix the docs or change the solution.

borisson_’s picture

@jiv_e I read your response there again and read the other comments as well, looks like Tim is picking this up over in #2825497: SystemMainBlock only functions when used by block.module, that seems like a better solution (making sure main content can only be placed trough the block system). In any case - we don't need 3 issues about this one usecase.

If we want to also use this issue, I'd suggest throwing the exception instead.

tim.plunkett’s picture

Status: Needs work » Closed (duplicate)
joseph.olstad’s picture

just fyi
patch still applies cleanly to 9.1.x

joseph.olstad’s picture

Status: Closed (duplicate) » Needs review

patch still applies cleanly to 9.1.x - triggering
patch still applies cleanly to 9.0.x - triggering
patch still applies cleanly to 8.9.x - triggering

marking this as needs review
setting other issue as duplicate. other issue patch no longer applies and fails testing.
this patch passed tests last time

tim.plunkett’s picture

Closing this one as a duplicate of #3164389: Enforce block plugins returning an array, an issue that has test coverage and only needs review.

tim.plunkett’s picture

Status: Needs review » Closed (duplicate)
joseph.olstad’s picture

Thanks, I got tripped up because the correct parent was not linked.
adding the correct parent duplicate issue