[ticket/16558] Improve viewonline page #6031
Conversation
Pages that are not handled by viewonline now shown as "Unrecognised Page" Admins see the URI of session table page PHPBB3-16558
There was a problem hiding this comment.
The overall point can be also reached with an extension, just for example like it's done with the Pages here https://github.com/phpbb-extensions/pages/blob/17a0438177c84a6d0be8a5b242bfa5abdc40cb28/event/listener.php#L134 or with this one https://github.com/rxu/detailed_viewonline (this one is by me, sorry).
And fingers crossed it won't break extensions as well :)
| @@ -835,6 +835,7 @@ | |||
| 'UNREAD_MESSAGES' => 'Unread messages', | |||
| 'UNREAD_POST' => 'Unread post', | |||
| 'UNREAD_POSTS' => 'Unread posts', | |||
| 'UNRECOGNISED_PAGE' => 'Unrecognised page', | |||
There was a problem hiding this comment.
I'm pretty sure that will confuse a lot of users and will generate lots of support topics like What does the Unrecognized page mean?.
There was a problem hiding this comment.
If that were a problem, would it be more sensible to omit unrecognised page reports to users other than admins?
There was a problem hiding this comment.
I don't think so as that would lead to even more confusing: like having 10 users online in stats you'll get f.e. 1 user on viewonline page for regular user.
There was a problem hiding this comment.
I don't understand why you prefer to display incorrect information rather than correct information?
Am I misunderstanding the purpose of the viewonline page?
| @@ -263,6 +263,55 @@ | |||
| case 'viewtopic': | |||
| $forum_id = $row['session_forum_id']; | |||
|
|
|||
| if (!$forum_id) | |||
| { | |||
| $array_match_index = 5; | |||
There was a problem hiding this comment.
There probably should be a cleaner way than match exactly index of 5.
There was a problem hiding this comment.
I put the constant there because it applies to several following regexs and it seemed easier to support if the regex gets changed in the future. alternatives welcome.
|
Personally, I don't see the need for this. |
Remove unsupport PHP syntax PHPBB3-16558
Fix memory leak PHPBB3-16558
Tend to agree. |
Fix overload of variables PHPBB3-16558
|
I also don't really see the need for this. Extensions that add their own pages can handle this (not that many actually add their own pages) and it's quite easy with an event. As for the core, well within the core itself there shouldn't be any unrecognized pages really, so this shouldn't really be in the core. |
|
This patch has nothing to do with extensions. It is fixing a deficiency in core phpBB that means that a range of core pages are not correctly identified on the view users page. |
phpBB/viewonline.php
Outdated
| break; | ||
| } | ||
| if ($auth->acl_get('a_')) | ||
| { | ||
| $location .= '<br>' . substr($row['session_page'], 0, 99); |
There was a problem hiding this comment.
This line is, um, weird. Shouldn't just be some arbitrary sub string. Also shouldn't be using HTML in PHP. Should instead be
$location .= nl2br("\n{$row['session_page']}");There was a problem hiding this comment.
I agree that it was bad practice to have HTML there, and I have removed it.
I felt that the substring is necessary because extremely long urls with trailing garbage (from bad bots??) caused the html formatting of the whole page to get broken
There was a problem hiding this comment.
This kind of thing should be done in the template ideally
There was a problem hiding this comment.
Well if you thought showing the URL would be of any use, trimming it down to only part of the URL makes it of very little use again. Not to mention a 99 character URL would probably still break layout in mobile. If you're worried about layout, then you should address that in the template with CSS like an overflow scroll for example. Or use CSS to have it wrap without breaking the layout. Or, also in Twig, use tags and functions like length and slice to only show it up to x num of chars, and add an ellipses to longer strings, and show the full url in a mouseover hover.
remove html from php file
Pages that are not handled by viewonline now shown as "Unrecognised Page"
Admins see the URI of session table page
PHPBB3-16558
Checklist:
Tracker ticket (set the ticket ID to your ticket ID):
https://tracker.phpbb.com/browse/PHPBB3-16558