Probable bug in views_exposed_form()

[markabur]

Description of the bug

In views_exposed_form(), $batch is not set/checked correctly.

Steps to reproduce

I'm not sure how to make this bug trigger an error!

Actual behavior

I was poking around for an unrelated issue and came across this:

function views_exposed_form($form, &$form_state) {
  // Don't show the form when batch operations are in progress.
  if ($batch = batch_get() && isset($batch['current_set'])) {
    return array(
      // Set the theme callback to be nothing to avoid errors in template_preprocess_views_exposed_form().
      '#theme' => '',
    );
  }

In PHP, && has higher precedence than =, which means that this is what's happening (causing $batch to be a boolean, not an array as expected):

[...]
  if ($batch = (batch_get() && isset($batch['current_set']))) {
[...]

Expected behavior

This is what should be happening instead:

[...]
  if (($batch = batch_get()) && isset($batch['current_set'])) {
[...]

Additional information

Here's the regex pattern I used to find this:

if \(\$[a-z_]+ = \S+ &&

There is one other occurrence in core. It's confusing but it doesn't cause any problems since $test_prefix isn't used anyplace:

if ($test_prefix = backdrop_valid_test_ua() && db_table_exists('tempstore')) {

[argiepiano]

You are right. The if statement as written will always result in false since $batch['current_set'] is never set.

In fact, in D7 Views:

• Version 3.24 had the same faulty check
• Version 3.25 fixed this mistake
• BUT, in version 3.26, this check was completely removed, meaning that probably is not needed.
• And here's the issue and the commit that removed this. In fact, fixing this the way you are suggesting may break Views Bulk Operations

We should try to update Views-related code to match D7. I bet there is an issue for this already. As a side note, it's kind of embarrassing to discover we are still using Views 3.24 in core (current version is 3.29).

EDIT: The rationale for completely removing that if block is that it has always been false and never executed, and that has caused no problems. In D7 they tried to "fix it" and they ended up breaking things.

[markabur]

Aha, how about that! Makes sense to delete it instead of fixing it. Here is the views update issue: #3685

[argiepiano]

That issue is pretty ancient. There are new commits to check.