Skip to content

Drale1/8.x/fix php 8 type error in block design regions or hidden regions in dxpr theme.theme#782

Open
drale1 wants to merge 1 commit into8.xfrom
drale1/8.x/Fix-PHP-8-TypeError-in-block_design_regions-or-hidden_regions-in-dxpr_theme.theme
Open

Drale1/8.x/fix php 8 type error in block design regions or hidden regions in dxpr theme.theme#782
drale1 wants to merge 1 commit into8.xfrom
drale1/8.x/Fix-PHP-8-TypeError-in-block_design_regions-or-hidden_regions-in-dxpr_theme.theme

Conversation

@drale1
Copy link
Copy Markdown
Collaborator

@drale1 drale1 commented Feb 27, 2026

Linked issues

Fix is related to the Drupal.org issue https://www.drupal.org/project/dxpr_theme/issues/3570233

Solution

This PR adds simple is_array() guards around theme_get_setting('hidden_regions') and theme_get_setting('block_design_regions') before passing their values into array_filter() in dxpr_theme.theme.

On PHP 8+, theme_get_setting() can return null for these settings on some configurations. Passing null directly into array_filter() causes a TypeError. With the guards in place, the theme safely skips applying the related logic when the settings are not defined, and pages render without PHP fatals.

Checklist

  • I have read the CONTRIBUTING.md document.
  • My commit messages follow the contributing standards and style of this project.
  • My code follows the coding standards and style of this project.
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Need to run update.php after code changes
  • Requires a change to end-user documentation.
  • Requires a change to developer documentation.
  • Requires a change to QA tests.
  • Requires a new QA test.
  • I have updated the documentation accordingly.
  • All new and existing tests passed.

@drale1
Copy link
Copy Markdown
Collaborator Author

drale1 commented Feb 27, 2026

Although the original reporter confirmed that the patch applies cleanly and fixes the issue in their local environment, I have not been able to reliably reproduce the TypeError myself.

Because this depends on specific configuration/upgrade paths (where theme_get_setting() returns null), I’d really appreciate some extra testing from other environments before this is merged, just to make sure there are no side effects in existing sites that don’t currently hit the error.

@drale1 drale1 requested a review from jjroelofs February 27, 2026 07:31
Copy link
Copy Markdown
Collaborator

@jjroelofs jjroelofs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix @drale1 — the is_array() guards for hidden_regions and block_design_regions look correct and are clearly needed for PHP 8+ compatibility. Nice catch.

However, there are a couple of issues with this PR:

1. Unrelated menu active trail changes not mentioned in PR description

The diff also removes the $is_front parameter from dxpr_theme_menu_set_active_trail() and drops the front page check ($is_front && $item_path === '/') — but this isn't mentioned anywhere in the PR description, commit messages, or linked issue #781.

Per CONTRIBUTING.md: "do not include changes to files not in this list unless absolutely necessary. When you do this you must explain why."

Can you clarify why these menu changes are included? Are they intentional or did they end up here by accident?

2. These menu changes already exist on 8.x

Looking at the current 8.x branch, the $is_front parameter was already removed in earlier merged PRs. Your branch appears to be based on an older 8.x commit, so these diffs are stale — they'll either conflict on merge or be no-ops.

3. "Fix space" commit should be squashed

The second commit (98f2a89 — "Fix space") should be squashed into the first to keep the history clean and atomic.

Requested action

Please rebase onto current 8.x — that should drop the menu-related diffs automatically and leave only the two is_array() guard changes, which are good to go. Squash the "Fix space" commit while you're at it.

@drale1 drale1 force-pushed the drale1/8.x/Fix-PHP-8-TypeError-in-block_design_regions-or-hidden_regions-in-dxpr_theme.theme branch from 98f2a89 to 8ee8a95 Compare March 18, 2026 07:57
@drale1
Copy link
Copy Markdown
Collaborator Author

drale1 commented Mar 18, 2026

@jjroelofs I investigated this and honestly can’t tell where the $is_front removal came from on this branch. It may have been accidentally introduced while I was working on another issue. Thanks for catching it.

I’ve removed those unrelated changes and rebuilt the PR as a clean, single-commit fix.

The current drupal-lint/stylelint failures don’t appear to be related to my changes. Would you like me to address them in this PR, or should we keep this PR focused and handle the lint issues separately?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants