[ticket/16696] Fix unsupported operand types in viewforum - PHP8 #6135
Conversation
3D-I
changed the title
| @@ -1027,7 +1027,7 @@ | |||
|
|
|||
| $template->assign_block_vars('topicrow', $topic_row); | |||
|
|
|||
| $pagination->generate_template_pagination($topic_row['U_VIEW_TOPIC'], 'topicrow.pagination', 'start', $topic_row['REPLIES'] + 1, $config['posts_per_page'], 1, true, true); | |||
| $pagination->generate_template_pagination($topic_row['U_VIEW_TOPIC'], 'topicrow.pagination', 'start', (int) $topic_row['REPLIES'] + 1, $config['posts_per_page'], 1, true, true); | |||
There was a problem hiding this comment.
What is the underlying problem here exactly?
There was a problem hiding this comment.
Is related to extensions that use the core.viewforum_modify_topicrow event.
There was a problem hiding this comment.
I've seen that. My question is why is this necessary? I would have assumed + implicitly casts anyway. Is that not the case?
There was a problem hiding this comment.
No, is not the case. It breaks in php 7 and PHP 8 is more explicit: unsupported operand types
I will provide a code example where you can see it in action later on.
The real issue is where the PR which caused this BC regression has been merged, anyway this simple fix solves the issue.
There was a problem hiding this comment.
There was a problem hiding this comment.
Moreover, there are potentially a lot of extensions which can break via that PR.
https://github.com/search?q=core.viewforum_modify_topicrow&type=code
My first impression it was to revert that PR.
There was a problem hiding this comment.
The problem is that extension authors have been using a value that was initially an integer value to change it to strings. As a result of that, casting to int would get rid of the warning but still somewhat has a potential of ending up with weird output. Anyway, weird output is still better than PHP warnings being output.
PHPBB3-16696
Checklist:
https://tracker.phpbb.com/browse/PHPBB3-16696