NullPage Errors when dealing with Parent Pages

[adrian]

It depends on the view - Tracy makes Adminer context aware.

But in the view you are seeing, it should look like this, in which case, it would be the checkbox to the left of "Table". But clearly that is missing from your screenshot. What I can tell is that based on that and some other missing UI elements, it looks like JS is disabled in your browser.

[adrian]

 

Quote

1 minute ago, ErikMH said:
I’m not sure what you’re suggesting?

Sorry if I wasn't clear - I thought you had some sites that were working with recent PW versions, so I thought if they were on the same machine you could try connect the non-working install to the DB of a site that is working. I really have no idea if this will uncover anything - just a very crude trial and error approach.

[ErikMH]

12 minutes ago, adrian said:

But in the view you are seeing, it should look like this, in which case, it would be the checkbox to the left of "Table". But clearly that is missing from your screenshot. What I can tell is that based on that and some other missing UI elements, it looks like JS is disabled in your browser.

Well now that’s very strange. I see exactly what I posted here, both in a WebKit browser and also in a Chromium-based browser — both of which have Javascript turned on. I thought perhaps there might be an invisible checkbox, but the pointer doesn’t change when I hover in that area, and clicking does nothing.

But it doesn’t really matter for the problem at hand; I won’t worry about this for now. Thanks!

[adrian]

Just one last comment - perhaps it's a JS error that is resulting in a similar issue to what I see when JS is off. Any browser dev console errors?

 

[ErikMH]

Oh, bingo — you called it!

sam.example.org/:1 Refused to load the script 'https://sam.example.org/cdn-cgi/scripts/7d0fa10a/cloudflare-static/rocket-loader.min.js' because it violates the following Content Security Policy directive: "script-src 'self' 'unsafe-inline' 'nonce-MzlhOGMzOWY0OTdkMDIxMmZkZDRiNTE0ZDBiYTg3YzI=' 'strict-dynamic'". Note that 'strict-dynamic' is present, so host-based allowlisting is disabled. Note that 'script-src-elem' was not explicitly set, so 'script-src' is used as a fallback.

I’m out of my ken here, @adrian. I see that something is set to be more secure than optimal, but I’ve no idea where to make a change. Cloudflare is mentioned — do you think that’s because it’s really where rocket-loader is served from, or because I have Cloudflare guarding my cloud-based development machine too well? Or is that not a fruitful line of inquiry?

This of course isn’t the serious problem that I need to resolve, but I’m glad to get these little points of friction worked out too. Thank you!

[adrian]

@ErikMH - it's for a different script, but this page (https://csplite.com/csp217/) explains how to either disable the script in the cloudflare dashboard, or how to adjust your CSP to prevent the error. It's definitely something you'll want to fix because JS errors have a habit of breaking lots of JS everywhere, hence the issues with adminer, but I am sure it won't be the only problem.

[ErikMH]

Thank you, @adrian, for that pointer. I’d had no idea that JS hadn’t been running. I’ll look into your example soon!

But the big news is that I’ve solved the problem! @ryan, @bernhard, & @kongondo — thanks to each of you, too, for your encouragement and good ideas!

Essentially, there is a bug, but it only presents itself when using findMany() instead of find(). As of 3.0.192, if you assemble a batch of pages with findMany() , and then call a function defined in a page-class definition on one of those pages, the $this variable in that function will have an empty parent property.

By contrast, in 3.0.191, the $this variable in that function will have the proper value in its parent property.

And also by contrast, in both 3.0.191 and in 3.0.192, if you use find() instead of findMany(), the function  will have the proper value in its parent property.

So, the bug shows up only in 3.0.192 or later and you’re using findMany() and you’re trying to find any parent-based information from the perspective of $this within a function of a page-class definition.

For my own use today, I’m perfectly OK using find() instead of findMany(), though there may come a time when I’m dealing with too many records for that to work gracefully. So I can upgrade beyond 3.0.184!

Thanks again to everyone for your help; I’ve learned a lot through this process. If an admin would like to break all this out (it has nothing to do with Ryan’s original post — I meant to reply initially to his announcement of 3.0.192!), I’d be grateful

[adrian]

@ErikMH - nice work narrowing it down to findMany(). Did you know there was a fix for a major issue with findMany() that was implemented for 3.0.195 - https://github.com/processwire/processwire/commit/1eb156f1aac64d388374c63cd401279f8d528481

Does upgrading to the latest dev fix the issue for you?

[ErikMH]

This is good to know about, @adrian — thanks for drawing my attention to it!

No, though, 3.0.195 behaves just like everything else for me beginning with 3.0.192. If I use findMany(), I have the problem as outlined above; if I use find(), everything’s fine.

[adrian]

16 minutes ago, ErikMH said:

No, though, 3.0.195 behaves just like everything else for me beginning with 3.0.192

That's a real shame - I thought we might have been onto something.

I just tested findMany() here, calling that same method I demo'd above and everything looks fine. Does this setup demonstrate the issue when you run it on your site?

[matjazp]

There are 20 commits between 3.0.191 and 3.0.192. It might be helpful to know which commit "broke" findMany()?

[kongondo]

5 hours ago, matjazp said:

There are 20 commits between 3.0.191 and 3.0.192. It might be helpful to know which commit "broke" findMany()?

It is interesting the this commit for findRaw() to support parent properties/fields in the return value happened in 3.0.192. Maybe it's just a coincidence but I find it interesting that the issue on this thread is about parent values and that commit, whilst for findRaw() was also about parent properties. 

[ErikMH]

OK, guys, it seems there was one more piece to the puzzle. Like @adrian, when I recreated the situation in a simple test environment running 3.0.192, I couldn’t recreate the problem. I should have tested this yesterday, but where I’d discovered that changing findMany() to find() in my code worked around the problem, I figured I’d found it.

So, the following conditions are necessary but insufficient:

1. PW 3.0.192 or greater (including the current version)
2. page classes
3. findMany() instead of find()

I believe I’ve found the final ingredient. I have tested this in a simplified environment and recreated the problem:

  4.  getPage()

In real life, I’ve rolled my own pagination (since I’d wanted to provide users with logarithmic-like controls for moving backwards and forwards through the site, and provide them with sign-post dates); this paginating has worked well. But it means that rather than a foreach loop I cycle through results in a for loop, and then assign the item with a getPage(). So, Adrian’s code looked like:

foreach($pages->findMany('template-basic-page') as $p) {
    d($p->id . ' : ' . $p->returnParentId());
}

but mine looks more like:

$items = $pages->findMany('template=Test');
for ($idx = 0; $idx < 1; $idx++):
	$p = $items->getPage($idx);
	echo ($p->identify());
endfor;

(Yes, that’s my test system, where I’ve hard-coded it to “loop” through just one test record; it’s sufficient to show the problem though:

ProcessWire\TestPage #279
id: 1025
name: 'test-record-a'
parent: ''
template: 'Test'
title: 'Test record A'
data: array (1)
'title' => 'Test record A'

parent there should = '1'.

I see that I can do the same thing by removing getPage() altogether:

$p = $items($idx);

In the grand scheme of things, I don’t know which is better; however, this assignation — whether implicit or via getPage(), seems to be the final piece: add this, and $this->parent() becomes a NullPage.

So, to sum up: all of this works fine in 3.0.150-or-so (where I developed it) on up through 3.0.191. As of .192, it breaks. Switching from findMany() to find() works around the problem. Sticking with findMany() and using the customary foreach() code pattern also works around the problem.

Phew! @kongondo, I agree that the commit you link to looks relevant. Do you think the (implicit or explicit) getPage() may call any of that code?

[adrian]

Ok, so I can confirm but it doesn't have anything to do with page classes - it appears to be all about getPage() together with findMany(). Here is a bare bones example with page classes disabled. Notice the change in parent between find and findMany:

 

[ErikMH]

Nicely done, @adrian!

[adrian]

I'd recommend you post a Github issue about this for Ryan, in case he doesn't read this thread.

[ErikMH]

1 hour ago, adrian said:

I'd recommend you post a Github issue about this for Ryan, in case he doesn't read this thread.

Excellent thought. I haven’t done this before, so hopefully I’ve got the details right:

findMany() combined with getPage() gives pages whose parents are (incorrectly) NullPage

[matjazp]

It looks like this commit is responsible: https://github.com/processwire/processwire/commit/fc0db2bb224075e7d6e4105619a2293a5ca4b7a0 more specifically PagesLoader.php on line 1176 where parent_id is unset.

[adrian]

And of course it has nothing to do with getPage() either :) Just all about findMany()

[ErikMH]

Well, but that can’t be right. You posted this, with findMany() working just fine yesterday:

So it looks like there’s something about getPage() (implicit or explicit), first(), and presumably last(), in combination with findMany(). I think.

[adrian]

3 hours ago, ErikMH said:

So it looks like there’s something about getPage() (implicit or explicit), first(), and presumably last(), in combination with findMany().

Looks like it: