2008-11-10, 01:52 AM
This bug was reported before several times but disregarded:
http://community.mybboard.net/thread-39298.html
http://community.mybboard.net/thread-35271.html
I'm reopening this bug because I believe I found more issues related to it as well as a way to fix the issues. My findings are all with MyBB 1.4.3 and they're all related to read / unread status of a forum that has one or more subforums and maybe even subsubforums.
This issue is quite complex and I'm sorry for making a mess here.
There are at least three issues:
1) Parent forum gets marked as 'unread' even though there is nothing new in it.
This happens if you post something in a subforum and your post is the latest posting in the parent forum. MyBB compared latest posting time against the time the parent forum was last read. If you read the subforum but not the parent forum the parent forum shows up as new.
2) Parent forum is marked as 'read' even though there is a subforum with unread postings in it.
This happens when the latest posting is read but another posting in another sibling subforum is not read.
3) When clicking 'mark forum as read' it does not mark (all) subforums as read. I have not investigated closer as to why that is yet but it's annoying after you fix 1) and 2) since then you can mark a forum as read, reload, and it's (correctly) unread again because the subforums didn't get marked as read.
A possible fourth issue is actually a feature request.
4) when unread / read markers work properly for subforums it would be nice to show those first in the forum list and not show 2 read subforums and have the unread subforums be grouped under "7 others".
What follows is my take at issues 1) and 2).
The issue is located in inc/features_forumlist.php functions build_forumbits() and get_forum_lightbulb().
build_forumbits() traverses through a forum and all its subforums, looking for the latest post. The purpose of this is mainly to be able to display the last post and author and time in the right column of the forum in the standard theme. Once the latest post is found, build_forumbits() passes it on to get_forum_lightbulb().
get_forum_lightbulb() uses the latest post and compares it with the users last read time of the forum. Depending on the result it will return a lightbulb in "on" state (unread) or "off" state (read).
This logic works fine for flat forum structures - the last post is newer than your last read time, the forum is unread, the last post is older than your read time, the forum is read.
This logic does not work for subforums, as it's perfectly possible to read the last post of a subforum without reading other new posts in the parent forum (and by doing so the parent forum gets marked as read too even though there are new unread postings in it). As well as the other way around, you can read the latest post in a subforum but because it uses the same latest post in the parent forum, it marks it as new even though it's already read.
In order to do it correctly, the get_forum_lightbulb() has to consider the last post directly in a forum, and not a newer last post of one of its subforums. At the same time, if a subforum is unread, the parent forum has to inherit the unread state too.
This change can be implemented at almost no additional CPU cost, because the code already collects all the information necessary and recurses through the forums, and compares the last post of each forum with the last post of the subforums in order to find the latest last post. In other words the information we need is already obtained by the current code but then ditched in favour of another subforums latest post.
Here are the changes I made to implement the new logic. In my tests it seemed to work fine but the code itself is not very nice (I'm not a PHP programmer ... I'm sorry). I'm confident that the issues 1.) through 4.) can be solved but I'm not confident in my code, it may still be buggy.
Diff:
Full code:
http://community.mybboard.net/thread-39298.html
http://community.mybboard.net/thread-35271.html
I'm reopening this bug because I believe I found more issues related to it as well as a way to fix the issues. My findings are all with MyBB 1.4.3 and they're all related to read / unread status of a forum that has one or more subforums and maybe even subsubforums.
This issue is quite complex and I'm sorry for making a mess here.
There are at least three issues:
1) Parent forum gets marked as 'unread' even though there is nothing new in it.
This happens if you post something in a subforum and your post is the latest posting in the parent forum. MyBB compared latest posting time against the time the parent forum was last read. If you read the subforum but not the parent forum the parent forum shows up as new.
2) Parent forum is marked as 'read' even though there is a subforum with unread postings in it.
This happens when the latest posting is read but another posting in another sibling subforum is not read.
3) When clicking 'mark forum as read' it does not mark (all) subforums as read. I have not investigated closer as to why that is yet but it's annoying after you fix 1) and 2) since then you can mark a forum as read, reload, and it's (correctly) unread again because the subforums didn't get marked as read.
A possible fourth issue is actually a feature request.
4) when unread / read markers work properly for subforums it would be nice to show those first in the forum list and not show 2 read subforums and have the unread subforums be grouped under "7 others".
What follows is my take at issues 1) and 2).
The issue is located in inc/features_forumlist.php functions build_forumbits() and get_forum_lightbulb().
build_forumbits() traverses through a forum and all its subforums, looking for the latest post. The purpose of this is mainly to be able to display the last post and author and time in the right column of the forum in the standard theme. Once the latest post is found, build_forumbits() passes it on to get_forum_lightbulb().
get_forum_lightbulb() uses the latest post and compares it with the users last read time of the forum. Depending on the result it will return a lightbulb in "on" state (unread) or "off" state (read).
This logic works fine for flat forum structures - the last post is newer than your last read time, the forum is unread, the last post is older than your read time, the forum is read.
This logic does not work for subforums, as it's perfectly possible to read the last post of a subforum without reading other new posts in the parent forum (and by doing so the parent forum gets marked as read too even though there are new unread postings in it). As well as the other way around, you can read the latest post in a subforum but because it uses the same latest post in the parent forum, it marks it as new even though it's already read.
In order to do it correctly, the get_forum_lightbulb() has to consider the last post directly in a forum, and not a newer last post of one of its subforums. At the same time, if a subforum is unread, the parent forum has to inherit the unread state too.
This change can be implemented at almost no additional CPU cost, because the code already collects all the information necessary and recurses through the forums, and compares the last post of each forum with the last post of the subforums in order to find the latest last post. In other words the information we need is already obtained by the current code but then ditched in favour of another subforums latest post.
Here are the changes I made to implement the new logic. In my tests it seemed to work fine but the code itself is not very nice (I'm not a PHP programmer ... I'm sorry). I'm confident that the issues 1.) through 4.) can be solved but I'm not confident in my code, it may still be buggy.
Diff:
--- original/mybb_1403.zip.dir/Upload/inc/functions_forumlist.php 2008-10-28 02:37:26.000000000 +0100
+++ mybb_1403.zip.dir/Upload/inc/functions_forumlist.php 2008-11-10 02:26:16.228899921 +0100
@@ -21,6 +21,7 @@
global $fcache, $moderatorcache, $forumpermissions, $theme, $mybb, $templates, $bgcolor, $collapsed, $lang, $showdepth, $plugins, $parser, $forum_viewers;
$forum_listing = '';
+ $bulb_was_on = 0;
// If no forums exist with this parent, do nothing
if(!is_array($fcache[$pid]))
@@ -36,6 +37,7 @@
$forums = $subforums = $sub_forums = '';
$lastpost_data = '';
$counters = '';
+ $bulb_is_on = 0;
// Get the permissions for this forum
$permissions = $forumpermissions[$forum['fid']];
@@ -71,6 +73,11 @@
{
$forum_info = build_forumbits($forum['fid'], $depth+1);
+ if($bulb_is_on == 0 && $forum_info['bulb'] == 1)
+ {
+ $bulb_was_on = $bulb_is_on = 1;
+ }
+
// Increment forum counters with counters from child forums
$forum['threads'] += $forum_info['counters']['threads'];
$forum['posts'] += $forum_info['counters']['posts'];
@@ -114,15 +121,20 @@
$parent_counters['viewers'] += $forum['viewers'];
}
+ // Get the lightbulb status indicator for this forum based on the lastpost
+ $lightbulb = get_forum_lightbulb($forum, $lastpost_data, $hideinfo, $bulb_is_on);
+
+ if($lightbulb['folder'] == "on")
+ {
+ $bulb_was_on = $bulb_is_on = 1;
+ }
+
// Done with our math, lets talk about displaying - only display forums which are under a certain depth
if($depth > $showdepth)
{
continue;
}
- // Get the lightbulb status indicator for this forum based on the lastpost
- $lightbulb = get_forum_lightbulb($forum, $lastpost_data, $hideinfo);
-
// Fetch the number of unapproved threads and posts for this forum
$unapproved = get_forum_unapproved($forum);
@@ -317,7 +329,8 @@
return array(
"forum_list" => $forum_list,
"counters" => $parent_counters,
- "lastpost" => $parent_lastpost
+ "lastpost" => $parent_lastpost,
+ "bulb" => $bulb_was_on
);
}
@@ -328,7 +341,7 @@
* @param array Array of information about the lastpost date
* @return array Array of the folder image to be shown and the alt text
*/
-function get_forum_lightbulb($forum, $lastpost, $locked=0)
+function get_forum_lightbulb($forum, $lastpost, $locked=0, $bulb_is_on=0)
{
global $mybb, $lang, $db, $unread_forums;
@@ -355,15 +368,16 @@
$forum_read = $mybb->user['lastvisit'];
}
+ $folder = "on";
+ $altonoff = $lang->new_posts;
+
// If the lastpost is greater than the last visit and is greater than the forum read date, we have a new post
- if($lastpost['lastpost'] > $forum_read && $lastpost['lastpost'] != 0)
+ if($forum['lastpost'] > $forum_read && $forum['lastpost'] != 0)
{
$unread_forums++;
- $folder = "on";
- $altonoff = $lang->new_posts;
}
// Otherwise, no new posts
- else
+ else if($bulb_is_on == 0)
{
$folder = "off";
$altonoff = $lang->no_new_posts;
Full code:
/**
* Build a list of forum bits.
*
* @param int The parent forum to fetch the child forums for (0 assumes all)
* @param int The depth to return forums with.
* @return array Array of information regarding the child forums of this parent forum
*/
function build_forumbits($pid=0, $depth=1)
{
global $fcache, $moderatorcache, $forumpermissions, $theme, $mybb, $templates, $bgcolor, $collapsed, $lang, $showdepth, $plugins, $parser, $forum_viewers;
$forum_listing = '';
$bulb_was_on = 0;
// If no forums exist with this parent, do nothing
if(!is_array($fcache[$pid]))
{
return;
}
// Foreach of the forums in this parent
foreach($fcache[$pid] as $parent)
{
foreach($parent as $forum)
{
$forums = $subforums = $sub_forums = '';
$lastpost_data = '';
$counters = '';
$bulb_is_on = 0;
// Get the permissions for this forum
$permissions = $forumpermissions[$forum['fid']];
// If this user doesnt have permission to view this forum and we're hiding private forums, skip this forum
if($permissions['canview'] != 1 && $mybb->settings['hideprivateforums'] == 1)
{
continue;
}
$plugins->run_hooks_by_ref("build_forumbits_forum", $forum);
// Build the link to this forum
$forum_url = get_forum_link($forum['fid']);
// This forum has a password, and the user isn't authenticated with it - hide post information
$hideinfo = false;
if($forum['password'] != '' && $mybb->cookies['forumpass'][$forum['fid']] != md5($mybb->user['uid'].$forum['password']))
{
$hideinfo = true;
}
$lastpost_data = array(
"lastpost" => $forum['lastpost'],
"lastpostsubject" => $forum['lastpostsubject'],
"lastposter" => $forum['lastposter'],
"lastposttid" => $forum['lastposttid'],
"lastposteruid" => $forum['lastposteruid']
);
// Fetch subforums of this forum
if(isset($fcache[$forum['fid']]))
{
$forum_info = build_forumbits($forum['fid'], $depth+1);
if($bulb_is_on == 0 && $forum_info['bulb'] == 1)
{
$bulb_was_on = $bulb_is_on = 1;
}
// Increment forum counters with counters from child forums
$forum['threads'] += $forum_info['counters']['threads'];
$forum['posts'] += $forum_info['counters']['posts'];
$forum['unapprovedthreads'] += $forum_info['counters']['unapprovedthreads'];
$forum['unapprovedposts'] += $forum_info['counters']['unapprovedposts'];
$forum['viewers'] += $forum_info['counters']['viewing'];
// If the child forums' lastpost is greater than the one for this forum, set it as the child forums greatest.
if($forum_info['lastpost']['lastpost'] > $lastpost_data['lastpost'])
{
$lastpost_data = $forum_info['lastpost'];
}
$sub_forums = $forum_info['forum_list'];
}
// If we are hiding information (lastpost) because we aren't authenticated against the password for this forum, remove them
if($hideinfo == true)
{
unset($lastpost_data);
}
// If the current forums lastpost is greater than other child forums of the current parent, overwrite it
if($lastpost_data['lastpost'] > $parent_lastpost['lastpost'])
{
$parent_lastpost = $lastpost_data;
}
if(is_array($forum_viewers) && $forum_viewers[$forum['fid']] > 0)
{
$forum['viewers'] = $forum_viewers[$forum['fid']];
}
// Increment the counters for the parent forum (returned later)
if($hideinfo != true)
{
$parent_counters['threads'] += $forum['threads'];
$parent_counters['posts'] += $forum['posts'];
$parent_counters['unapprovedposts'] += $forum['unapprovedposts'];
$parent_counters['unapprovedthreads'] += $forum['unapprovedthreads'];
$parent_counters['viewers'] += $forum['viewers'];
}
// Get the lightbulb status indicator for this forum based on the lastpost
$lightbulb = get_forum_lightbulb($forum, $lastpost_data, $hideinfo, $bulb_is_on);
if($lightbulb['folder'] == "on")
{
$bulb_was_on = $bulb_is_on = 1;
}
// Done with our math, lets talk about displaying - only display forums which are under a certain depth
if($depth > $showdepth)
{
continue;
}
// Fetch the number of unapproved threads and posts for this forum
$unapproved = get_forum_unapproved($forum);
if($hideinfo == true)
{
unset($unapproved);
}
// Sanitize name and description of forum.
$forum['name'] = preg_replace("#&(?!\#[0-9]+;)#si", "&", $forum['name']); // Fix & but allow unicode
$forum['description'] = preg_replace("#&(?!\#[0-9]+;)#si", "&", $forum['description']); // Fix & but allow unicode
$forum['name'] = preg_replace("#&([^\#])(?![a-z1-4]{1,10};)#i", "&$1", $forum['name']);
$forum['description'] = preg_replace("#&([^\#])(?![a-z1-4]{1,10};)#i", "&$1", $forum['description']);
// If this is a forum and we've got subforums of it, load the subforums list template
if($depth == 2 && $sub_forums)
{
eval("\$subforums = \"".$templates->get("forumbit_subforums")."\";");
}
// A depth of three indicates a comma separated list of forums within a forum
else if($depth == 3)
{
if($donecount < $mybb->settings['subforumsindex'])
{
$statusicon = '';
// Showing mini status icons for this forum
if($mybb->settings['subforumsstatusicons'] == 1)
{
$lightbulb['folder'] = "mini".$lightbulb['folder'];
eval("\$statusicon = \"".$templates->get("forumbit_depth3_statusicon", 1, 0)."\";");
}
// Fetch the template and append it to the list
eval("\$forum_list .= \"".$templates->get("forumbit_depth3", 1, 0)."\";");
$comma = ', ';
}
// Have we reached our max visible subforums? put a nice message and break out of the loop
++$donecount;
if($donecount == $mybb->settings['subforumsindex'])
{
if(subforums_count($fcache[$pid]) > $donecount)
{
$forum_list .= $comma.$lang->sprintf($lang->more_subforums, (subforums_count($fcache[$pid]) - $donecount));
}
}
continue;
}
// Forum is a category, set template type
if($forum['type'] == 'c')
{
$forumcat = '_cat';
}
// Forum is a standard forum, set template type
else
{
$forumcat = '_forum';
}
if($forum['linkto'] == '')
{
// No posts have been made in this forum - show never text
if(($lastpost_data['lastpost'] == 0 || $lastpost_data['lastposter'] == '') && $hideinfo != true)
{
$lastpost = "<div style=\"text-align: center;\">{$lang->lastpost_never}</div>";
}
elseif($hideinfo != true)
{
// Format lastpost date and time
$lastpost_date = my_date($mybb->settings['dateformat'], $lastpost_data['lastpost']);
$lastpost_time = my_date($mybb->settings['timeformat'], $lastpost_data['lastpost']);
// Set up the last poster, last post thread id, last post subject and format appropriately
$lastpost_profilelink = build_profile_link($lastpost_data['lastposter'], $lastpost_data['lastposteruid']);
$lastpost_link = get_thread_link($lastpost_data['lastposttid'], 0, "lastpost");
$lastpost_subject = $full_lastpost_subject = $parser->parse_badwords($lastpost_data['lastpostsubject']);
if(my_strlen($lastpost_subject) > 25)
{
$lastpost_subject = my_substr($lastpost_subject, 0, 25)."...";
}
$lastpost_subject = htmlspecialchars_uni($lastpost_subject);
$full_lastpost_subject = htmlspecialchars_uni($full_lastpost_subject);
// Call lastpost template
if($depth != 1)
{
eval("\$lastpost = \"".$templates->get("forumbit_depth{$depth}_forum_lastpost")."\";");
}
}
$forum_viewers_text = '';
if($mybb->settings['showforumviewing'] != 0 && $forum['viewers'] > 0)
{
if($forum['viewers'] == 1)
{
$forum_viewers_text = $lang->viewing_one;
}
else
{
$forum_viewers_text = $lang->sprintf($lang->viewing_multiple, $forum['viewers']);
}
$forum_viewers_text = "<span class=\"smalltext\">{$forum_viewers_text}</span>";
}
}
// If this forum is a link or is password protected and the user isn't authenticated, set lastpost and counters to "-"
if($forum['linkto'] != '' || $hideinfo == true)
{
$lastpost = "<div style=\"text-align: center;\">-</div>";
$posts = "-";
$threads = "-";
}
// Otherwise, format thread and post counts
else
{
$posts = my_number_format($forum['posts']);
$threads = my_number_format($forum['threads']);
}
// Moderator column is not off
if($mybb->settings['modlist'] != 0)
{
$done_moderators = array();
$moderators = '';
// Fetch list of moderators from this forum and its parents
$parentlistexploded = explode(',', $forum['parentlist']);
foreach($parentlistexploded as $mfid)
{
// This forum has moderators
if(is_array($moderatorcache[$mfid]))
{
// Fetch each moderator from the cache and format it, appending it to the list
foreach($moderatorcache[$mfid] as $moderator)
{
if(in_array($moderator['uid'], $done_moderators))
{
continue;
}
$moderators .= "{$comma}<a href=\"".get_profile_link($moderator['uid'])."\">".htmlspecialchars_uni($moderator['username'])."</a>";
$comma = ', ';
$done_moderators[] = $moderator['uid'];
}
}
}
$comma = '';
// If we have a moderators list, load the template
if($moderators)
{
eval("\$modlist = \"".$templates->get("forumbit_moderators")."\";");
}
else
{
$modlist = '';
}
}
// Descriptions aren't being shown - blank them
if($mybb->settings['showdescriptions'] == 0)
{
$forum['description'] = '';
}
// Check if this category is either expanded or collapsed and hide it as necessary.
$expdisplay = '';
$collapsed_name = "cat_{$forum['fid']}_c";
if(isset($collapsed[$collapsed_name]) && $collapsed[$collapsed_name] == "display: show;")
{
$expcolimage = "collapse_collapsed.gif";
$expdisplay = "display: none;";
$expaltext = "[+]";
}
else
{
$expcolimage = "collapse.gif";
$expaltext = "[-]";
}
// Swap over the alternate backgrounds
$bgcolor = alt_trow();
// Add the forum to the list
eval("\$forum_list .= \"".$templates->get("forumbit_depth$depth$forumcat")."\";");
}
}
// Return an array of information to the parent forum including child forums list, counters and lastpost information
return array(
"forum_list" => $forum_list,
"counters" => $parent_counters,
"lastpost" => $parent_lastpost,
"bulb" => $bulb_was_on
);
}
/**
* Fetch the status indicator for a forum based on its last post and the read date
*
* @param array Array of information about the forum
* @param array Array of information about the lastpost date
* @return array Array of the folder image to be shown and the alt text
*/
function get_forum_lightbulb($forum, $lastpost, $locked=0, $bulb_is_on=0)
{
global $mybb, $lang, $db, $unread_forums;
// This forum is closed, so override the folder icon with the "offlock" icon.
if($forum['open'] == 0 || $locked)
{
$folder = "offlock";
$altonoff = $lang->forum_locked;
}
else
{
// Fetch the last read date for this forum
if($forum['lastread'])
{
$forum_read = $forum['lastread'];
}
else
{
$forum_read = my_get_array_cookie("forumread", $forum['fid']);
}
if(!$forum_read)
{
$forum_read = $mybb->user['lastvisit'];
}
$folder = "on";
$altonoff = $lang->new_posts;
// If the lastpost is greater than the last visit and is greater than the forum read date, we have a new post
if($forum['lastpost'] > $forum_read && $forum['lastpost'] != 0)
{
$unread_forums++;
}
// Otherwise, no new posts
else if($bulb_is_on == 0)
{
$folder = "off";
$altonoff = $lang->no_new_posts;
}
}
return array(
"folder" => $folder,
"altonoff" => $altonoff
);
}
