Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broken page list in "move page" and "select page" widgets (server error) #6020

Closed
fvsch opened this issue Nov 29, 2023 · 4 comments · Fixed by #6029
Closed

Broken page list in "move page" and "select page" widgets (server error) #6020

fvsch opened this issue Nov 29, 2023 · 4 comments · Fixed by #6029
Assignees
Labels
type: bug 🐛 Is a bug; fixes a bug
Milestone

Comments

@fvsch
Copy link

fvsch commented Nov 29, 2023

Description

I’m testing an update of a 3.9 site to 4.0 and it looks like the page list widget used in a few places, namely in the "Move page" dialog and in the page selection widget of the Retour for Kirby plugin, breaks with a server error:

{
    "code": 500,
    "error": "array_merge(): Argument #3 must be of type array, bool given"
}

Some URLs for GET requests from the Panel that show this exact error:

  • http://localhost:8000/panel/site/tree?parent=%2Fsite
  • http://localhost:8000/panel/site/tree?move=%2Fpages%2Fcontact&parent=%2Fsite

Expected behavior

The request to /panel/site/tree doesn't fail.

Screenshots

Screenshot 2023-11-29 at 11 12 13

To reproduce

I don't have steps to reproduce or a minimal reproduction yet.

I understand this is very little information to go with, and I suspect it's something in my setup (otherwise there would be issues for this filed already). I’m not confident I can make a simple repro through trial and error.

Any pointers on how I could investigate this?

I have 'debug' => true, 'whoops' => true in my config, but those Panel API requests don't show more information about where the issue occurred.

Your setup

Kirby Version

4.0.0

Console output
See above for the failing API request. There's another JS error after that, which I suspect comes from not handling the error response from the API when rendering the UI.

Your system (please complete the following information)

  • Device: M1 Mac
  • OS: macOS
  • Browser: Chrome
  • Version: 119
@fvsch
Copy link
Author

fvsch commented Nov 29, 2023

Alright, I found the issue.

Using XDebug, I found where the error was generated:

kirby/src/Panel/Model.php

Lines 115 to 120 in 40eae8e

// merge with defaults and blueprint option
$settings = array_merge(
$this->imageDefaults(),
$settings ?? [],
$this->model->blueprint()->image() ?? [],
);

The issue was that I had one page blueprint (site/blueprints/pages/error.yml) which had image: false at the root:

title: Error page
image: false

sections:
  fields:
    #

This faulty value (probably the result of writing blueprints through a lot of trial and error ^^) gets returned as-is by $this->model->blueprint()->image(), and since false ?? [] is false, we end up with a PHP error.

So this is definitely my fault and not a bug per se, but this was very hard to identify. Could something be improved here?

There is some blueprint data normalization going on here, maybe there should be more?

// normalize and translate the title
$props['title'] ??= ucfirst($props['name']);
$props['title'] = $this->i18n($props['title']);
// convert all shortcuts
$props = $this->convertFieldsToSections('main', $props);
$props = $this->convertSectionsToColumns('main', $props);
$props = $this->convertColumnsToTabs('main', $props);
// normalize all tabs
$props['tabs'] = $this->normalizeTabs($props['tabs'] ?? []);

@fvsch
Copy link
Author

fvsch commented Nov 29, 2023

By the way I checked the cookbook (because earlier in the year I had copied page blueprint examples from the cookbook that were triggering bugs in Kirby 3.9, so I was wondering if that was from a faulty example). Couldn't find an instance of image: false for a page blueprint. So it's probably just my mistake.

@distantnative
Copy link
Member

Will need look into this as these lines

kirby/src/Panel/Model.php

Lines 101 to 104 in 40eae8e

// completely switched off
if ($settings === false) {
return null;
}
should in theory cover the case you describe.

@distantnative
Copy link
Member

Got it, it's not the $settings one but the third argument.

@distantnative distantnative self-assigned this Nov 30, 2023
@distantnative distantnative added the type: bug 🐛 Is a bug; fixes a bug label Nov 30, 2023
@distantnative distantnative added this to the 4.1.0 milestone Nov 30, 2023
@distantnative distantnative linked a pull request Nov 30, 2023 that will close this issue
5 tasks
@bastianallgeier bastianallgeier changed the title [v4] Broken page list in "move page" and "select page" widgets (server error) Broken page list in "move page" and "select page" widgets (server error) Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug 🐛 Is a bug; fixes a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants