Hello, i have recently updated my php to 8.0 and realized a exposed filter changed how it works.
I'm using better exposed filters over the views, but i traced the issue to the views module.
When i have a boolean operator filter, and better exposed filter module assigns a default value of 0 (false), the "acceptExposedInput" method in Drupal\views\Plugin\views\filter\FilterPluginBase behaves differently.
At line 1507 of FilterPluginBase.php there is this condition, which evaluates to true (0=='string') in php7.4 and to false in php 8.0.
if ($value == 'All' || $value === []) {
return FALSE;
}
Also lines 111, 112 and 113 at Drupal\views\Plugin\views\filter\BooleanOperator look redundant to me.
Kind regards and many thanks for your awesome work.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | 3259090-15.patch | 2.64 KB | lendude |
| #15 | 3259090-15-TEST_ONLY.patch | 2.02 KB | lendude |
Comments
Comment #2
pandepoulus commentedComment #3
pandepoulus commentedComment #4
cilefen commentedWhat should be done here? Should you pick another default value?
Comment #5
pandepoulus commentedWell, booleanOperator filters with a default value of false are not filtering in php 7.4, which i think is an error, but are working properly with php 8.0.
probably just changing the condition to strcmp($value,'All') == 0 whould lead to same results regarding the php version.
Not sure something has to be done as i think is working fine in php 8.0
.
Comment #6
cilefen commentedYes, this is an interesting one.
Comment #7
cilefen commentedComment #8
nagy.balint commentedWe see the same problem on PHP8.
Its indeed related to https://www.php.net/manual/en/migration80.incompatible.php
strcmp likely wont work, as the value can be array as well, and strcmp needs a string.
Comment #9
mr.york commentedI created hotfix.
Comment #10
lendudeArgh!! It's the return of #1381140: A #default_value = 0 for #type radios checks all radios. Run away!!
Comment #12
jaims-dev commentedI can confirm that this is still an issue, and I can also confirm that the patch solved it.
It believe it should be reprioritazed as 'major' because for one particular site, views went from working to not working well (i.e. omitting rows that should be displayed) when updating the site from php7.4 to php8.1 (Drupal version is 9.4.8). As many sites will be updated to php8+ in the next months, this can really be considered an important issue.
Also, tagging this as RTBC
Comment #13
alexpottI can confirm the issue and that the fix looks okay... see https://3v4l.org/1t3f1
Ideally we'd have automated test coverage that would have caught this issue. But obviously we don't as this would have been fixed when we started testing on PHP 8. I think we should take this opportunity to add the missing coverage.
Comment #14
lendudeHere is a test for this, doing this in a functional test is very involved, so simple unit test
Lets see if this give the expected pass/fail. The test only patch is the interdiff
Comment #15
lendudeAnd lets try that again....
Comment #17
andypostThank you!
Comment #18
lendudeAlso ran the 'test only' on PHP7.4 and as expected it goes green there, so this seem to catch the bug as expected.
Comment #19
alexpottNice test.
Committed and pushed 812ec978db to 10.1.x and e11c30b58a to 10.0.x and ee3fedcd24 to 9.5.x. Thanks!
I think 9.4.x has had it's last bugfix release so this goes in 9.5.x