Array to string conversion in views_arg_load

[olafgrabienski]

Description of the bug

If you have two views with the same URL path, you get the following message on certain pages:

Notice: Array to string conversion in views_arg_load() (line 495 of /path-to-backdrop/core/modules/views/views.module).

Steps To Reproduce

To reproduce the behavior:

1. Go to admin/config/development/logging, and choose to display All messages.
2. Go to admin/structure/views, and enable the view Taxonomy term.
3. Clone the Taxonomy term view, and save it. (Do not change the path of the Page view.)
4. Create a Post, add a Title and at least one Tag, and save the Post.
5. Click one of the Tag links of the Post.

Actual behavior

Two lines of the Array to string conversion in views_arg_load() message.

Expected behavior

No message.

Additional information

Add any other information that could help, such as:

• Backdrop CMS version: 1.23.0
• Tested on a Backdrop demo site

[olafgrabienski]

This issue was fixed in Drupal 7, but somehow the fix didn't make it in Backdrop.

Drupal

• Issue report
• Duplicate issue with more information
• Patch and commit
• Views release 7.x-3.10 with the commit

Backdrop

• Crossports, Views commits from 7.x-3.9 to 7.x-3.11 (without the commit in question)

[olafgrabienski]

During an upgrade from D7 to Backdrop, the issue lead to a serious problem. After running core/update.php successfully, I went to the Home page and got the Array to string conversion in views_arg_load() message on a blank page together with the following notices and error:

Notice: Undefined index: MY_DISPLAY_ID in view->choose_display() (line 531 of /path-to-backdrop/core/modules/views/includes/view.inc). Notice: Trying to get property 'handler' of non-object in view->choose_display() (line 531 of /path-to-backdrop/core/modules/views/includes/view.inc). Error: Call to a member function access() on null in view->choose_display() (line 531 of /path-to-backdrop/core/modules/views/includes/view.inc).

As the error appeared even on the Login page, I was not able to log into the Backdrop site.

I'm not sure about the reason for the additional notices and thus the error (probably a particular Views display configuration on the upgraded D7 site). They happen however due to the array to string conversion in views_arg_load(), and I was able to get my Backdrop site running again by applying the D7 patch mentioned in https://github.com/backdrop/backdrop-issues/issues/5869#issuecomment-1331963382 to the Views module in Backdrop.

[argiepiano]

@olafgrabienski, I'm unable to reproduce the error you first described in this issue. I have done the following:

• Enabled the taxonomy_term View in a fresh installation of Backdrop
• Cloned that view. The clone's name is clone_of_taxonomy_term
• I added a couple of Posts with the same tag
• I visited the path of the view as in taxonomy/term/1

I get no error. The clone of the view is the one being invoked.

Is there a missing step in your description?

[EDIT: I also tried clicking the tag, which opens the same view through its alias, no errors]

[EDIT 2: Also tried PHP 8.1. No errors]

[olafgrabienski]

Thanks for trying, @argiepiano ! I missed indeed a step. In case you didn't change the Logging settings:

• Go to admin/config/development/logging, and choose to display All messages. (I've now added that step to the issue description.)

Does that change anything?

[indigoxela]

I can reproduce it! Followed the exact steps (on php 8.1) and ended up with:

    Warning: Array to string conversion in views_arg_load() (line 495 of ...core/modules/views/views.module).
    Error: Call to a member function bundle() on string in EntityBundleLayoutAccess->checkAccess() (line 77 of ...core/modules/layout/plugins/access/entity_bundle_layout_access.inc).

Note: for the second one - the Error - we already have #5809. Applying the other patch, reloading the page, and the

Warning: Array to string conversion in views_arg_load() (line 495 of ...core/modules/views/views.module).

thing is still there.

EDIT: obviously the layout I needed to reproduce the other bug was still on. But even if it's off, the warnings persist.

[argiepiano]

Thanks @olafgrabienski. Yes, I had errors on. For some reason I couldn't reproduce in my local, but I've spun a demo site through backdropcms.org and can see the error now. The patch you linked seems good - do you want to submit a PR for this?

[indigoxela]

The patch you linked seems good

You might want to review it (over there) :wink:

... do you want to submit a PR for this?

Can do (not today, though), but I'm also happy to review yours if you provide one. :smiley:

[argiepiano]

Haha I was referring to @olafgrabienski's linked patch, and inviting him to submit a PR 😁

[olafgrabienski]

Thanks for the invitation 😄 I'll submit a PR, probably tomorrow.

[olafgrabienski]

Here is the Pull Request, all checks passed: https://github.com/backdrop/backdrop/pull/4263

[argiepiano]

Thanks a lot for providing the PR @olafgrabienski! I've been looking a bit into it. It does its job well. I'm going to mark it WFM, but want to take a closer look at the code.

I believe views_load_args() and the way Views handles the menu page arguments may need a bit of a review - I understand why we are getting an array of display_ids - it's possible to have two displays of the same view that use same path with different access restrictions (e.g. one for anonymous, and one for authenticated users). BUT currently the function is getting the display_id of a different View, which just doesn't make sense (it's getting the display id page from the clone_of_taxonomy_term view, and a second page from the original taxonomy_term). But it's getting only one views $name - clone_of_taxonomy_term. This may need opening another issue.

[olafgrabienski]

I'm going to mark it WFM, but want to take a closer look at the code.

@argiepiano Thanks for marking the PR! Have you had a chance to look into the code again, and/or do you have a suggestion how to proceed?

[klonos]

Thanks for filing the issue, tracking down the 7.x change, and then filing the PR @olafgrabienski and sorry for having to wait this long for a code review.

The PR looks good, but can you please also update the docblock of the function to match the 7.x version?

Speaking of which, the docblock in 7.x has this:

 * @param string $display_id
 *   The display id that will be loaded for this menu item.

But in the code we are checking if that parameter is an array, which is confusing. Can we at least add an inline comment to explain why we are anticipating an array? Ideally though we'd change the docblock to say @param string | array $display_id, but we'd need to figure out something to go along as a comment/explanation.

[olafgrabienski]

@klonos I'll try my best, but will need some advice, as I'm not really familiar with the code logic. The more concrete the suggestions the better!

[klonos]

... The more concrete the suggestions the better!

Sure @olafgrabienski 👍🏼 ...for now, simply copying the docblock from the 7.x version would do. The only difference would be to change @param string $display_id to @param string | array $display_id and for now leave the description of that parameter as is. We'll figure out what needs to be written exactly to describe when $display_id might be an array.

[olafgrabienski]

Thank you very much, @klonos – I've updated the PR accordingly. Can you have another look?

Btw, the Coding standards test failed, but looks unrelated to my eyes. What do you think?

[klonos]

Thanks @olafgrabienski 🙏🏼 ...there's an additional line at the top of the docblock in the 7.x version, which we should include, as it is common to use a single line that summarizes functions. Docblocks should also ideally have a @return statement, but looking at the code, I understand why this one doesn't 😅 (too much going on).

You did great, but we now need to decide on wording and tweak final things in the docblock so that it's not such a mess. Then this will be ready to be merged.

[olafgrabienski]

@klonos and @kiamlaluno - Thanks for your suggestions here and in the PR. Unfortunately, I don't have any time to adapt the PR at the moment, and I'm not able to judge some questions. In the meantime, if one of you is up to it, feel free to take the PR over.

[herbdool]

I think this is ready to merge. Code matches the D7 commit and it's been tested. I made fixes to the docblock.

[quicksketch]

I merged https://github.com/backdrop/backdrop/pull/4263 into 1.x and 1.27.x. Thanks @olafgrabienski, @argiepiano, @indigoxela, @kiamlaluno, @klonos, and @herbdool!