Doubling of page tree when specifying an id - new bug introduced with the PW 3.0.8 and the smarter page tree.

[adrianbj]

If you have opened up a branch and then visit a link like this: http://pwtest.dev/processwire/page/?id=xxxx where the xxxx is the id of the parent page of the branch you previously opened, the child branches will be duplicated.

This is because of: $input->cookie->pagelist_open - if you remove the page id from the cookie string it behaves normally.

I know that visiting a branch using a URL like above is unusual, but I use it in the AdminRestrictBranch module. I have figured out a quick hack in the module to help out @SteveB, but I haven't committed yet as I am hoping this is something you might consider fixing in the core.

This is my hack:

    //set pagelist root to the correct parent based on the "How to match user to branch settings"
    public function setBranchRoot() {

        //set parent page of branch
        $_GET['id'] = $this->branchRootParentId;

        //if pagelist_open cookie is set and it matches the defined parent,
        //then need this to prevent doubling of page branch
        if(isset(wire('input')->cookie->pagelist_open)) {
            foreach(explode(',',wire('input')->cookie->pagelist_open) as $pageOpen) {
                if(current(explode("-", $pageOpen)) == $this->branchRootParentId) {
                    wire('input')->cookie->pagelist_open = str_replace($pageOpen, '', wire('input')->cookie->pagelist_open);
                }
            }
        }

        //if open get variable is set and it matches the defined parent,
        //then need this to prevent doubling of page branch
        if(isset($_GET['open']) && $_GET['open'] == $this->branchRootParentId) wire('input')->get->open = null;
    }

I was already resetting $input->get->open to null for previous versions of PW and I am ok with this, but the new hack for fixing the pagelist_open cookie is not ideal because it breaks the ability of the page tree to remember what was open. Do you think this could be fixed in the core? Maybe the $input->get->open issue could be considered a bug in the core also?

Please let me know if you need any more info about this.

[rolandtoth]

Having the same issue now on the second site (using the module AdminRestrictBranch). The above fix solves the issue in question but an official fix woud be better.

[adrianbj]

Thanks for chiming in on this @rolandtoth - I might need to add that "fix" to the module for the moment because at the moment it really isn't functional on PW 3 without it. Hopefully Ryan can implement a core fix soon though so that we can have tree open status as well.

[adrianbj]

I think I have a better fix for this. Turns out I was misunderstanding what was causing the duplication. The new fix seems to allow remembering of what was open in the tree. It was actually just a matter of removing the home page from the pagelist_open cookie: https://github.com/adrianbj/AdminRestrictBranch/blob/master/AdminRestrictBranch.module#L194

I might be getting a bit OT, here but I think most of the problem stems from the fact that the pagelist_open cookie isn't deleted when the user logs out, so when a different user logs in that has a branch restriction, the cookie still tries to open the homepage. This is a general question - should cookies be user specific, or be deleted on logout? This may not actually be a problem in the real world because it is unlikely that different users will log in on the same machine/browser, but I still think PW needs to consider it.

[ryancramerdesign]

Thanks Adrian, I've pushed a fix for this.

[adrianbj]

Thank Ryan - unfortunately I am not sure that is desired behavior - I think it should still open any sub-branches under the page with the specified ID. I think the approach I mention above: https://github.com/ryancramerdesign/ProcessWire/issues/1774#issuecomment-220597150 is better - I am not convinced it is completely perfect (I think @rolandtoth might still be having some issues). Maybe it needs to remove not just the homepage from the pagelist_open cookie - maybe it also needs to remove any other pages that are also above the specified page. Regardless, unfortunately I think the current fix is actually a bit of a step back - no duplication is essential (which it does achieve), but not showing previously opened pages is not ideal.

Did you have any thoughts on the user specific cookie thought?

@rolandtoth - did you have any other thoughts on this change and/or what would be preferable?

[rolandtoth]

I haven't upgraded my sites to the latest version to check - I'll get back when I find something.

[adrianbj]

Hey Ryan,

With PW 3.x about to be released, I think it is better to reverse the fix you implemented above based on my last comment because now it doesn't remember the status of the page tree at all, whereas I think my current fix in AdminRestrictBranch is working fine. Thanks.