Subforum read/unread inconsistencies [R]

[frostschutz]

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:

PHP Code:

--- 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:

PHP 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
    );
} 


[Ryan Gordon]

We don't read any farther into it because it would be too processor intensive. That is why this is being marked a bogus and the other one was too.

This was identified in alpha builds of MyBB 1.4's development. There just isn't a way to fix all these issues without causing others and in a non-intensive way on the server.

[frostschutz]

(11-10-2008 01:56 AM)Ryan Gordon Wrote:  We don't read any farther into it because it would be too processor intensive.

I find that hard to believe. The changes I made do not change CPU load - the forums are already traversed recursively by the current implementation and all my change does is actually use the information that is obtained this way. And the other issues are issues even present without fixing the read / unread state properly - it would be nice (and cost virtually nothing) to mark all subforums read when I mark a forum as read and put unread subforums first in the overview (if done right, also at next to nothing cost).

I don't care wether or not you fix this as I don't plan on using Subforums much either way. It's just an issue that came up while evaluating MyBB for our purposes (small multi lingual forums). I guess MyBB has bigger forums who would like a proper subforum handling.

It's a shame to not fix something when a good way to fix it exists (and I'm not talking about my code here, the idea should be implemented by a more experienced PHP programmer).

[Ryan Gordon]

I haven't looked into your fix yet but the tracking system for this is a lot more complex then what I think you understand of it.

I am all up for fixing it if there is a good fix but it doesn't seem like your fix is a proper one.

[frostschutz]

My fix definitely isn't a good one in terms of the code I write - I'm a programmer but not a PHP programmer - and I only did minimal modifications to the existing function just to see wether or not it's possible to fix at all - and as a result there is a bit of awkward variable handling there - but I think it is possible to fix the subforum read/unread handling.

My code is intended to be more of like a 'proof of concept', of course it's possible that I overlooked something and it doesn't work at all after all, in which case I'd like to be proven otherwise, i.e. a comment why the approach can't work instead of just "too much CPU" would be nice.

If the idea works, there is lots of room for optimization regarding the implementation. It can be rewritten in a way that goes better with the idea of looking at the top post of each forum instead of the very latest subforum post.

[Ryan Gordon]

(11-10-2008 02:27 AM)frostschutz Wrote:  My code is intended to be more of like a 'proof of concept', of course it's possible that I overlooked something and it doesn't work at all after all, in which case I'd like to be proven otherwise, i.e. a comment why the approach can't work instead of just "too much CPU" would be nice.

I would except most of us just don't have the time these days.

[frostschutz]

Ok, with some further tests, I found out that my proposal is incomplete.

What the current logic does to mark a forum as 'on' / new:
- determine the last post of the parent forum
- for each subforum, if the subforums last post is newer, use the subforums last post instead
- when lastpost timestamp > users forumsread timestamp, the forum bulb is 'on'

Problem:
When the subforums users forumsread timestamp > lastpost timestamp (i.e. the post is read in the subforum), yet the parent forum gets marked as unread if the parent forums users forumsread is < lastpost timestamp. You can't simply mark it as 'off' when the subforum is read though as there may be other new posts in another subforum or the parent forum itself.

The my logic I tried to implement marks a forum as 'on' / new when:
- the lastpost timestamp > users forumsread timestamp (so far identical to the old, it works as long as there is no subforum)
- or when a subforum is marked as 'on'

My intention here was that this way, you would get a path of bulbs that are 'on', so for example if the Subsubsubsubforum #47 is on, the Parent forum, subforum #4, subsubforum #13, etc. would all be on too, leading you to the forum that contains the new posting, i.e. * Parent > * Subforum > * Subsubforum etc. and once you'd seen the new posting in Subsubsubforum #47 the bulbs would all be off (if there are no other 'on' subforums).

This is consistent behaviour but for it to work completely, the 'mark forum as read' function would have to be changed in a way that also marks all subforums as read.

This isn't necessary with the old logic, i.e. the old logic allows marking the parent forum as read (which just sets the timestamp for the parent forum), while bulbs in subforums stay 'on'. In my logic that does not work because the parent forum gets marked as 'on' when a sub forum is marked as 'on'.

Personally I don't think it's a good idea to mark a parent forum as 'off' when a subforum is still 'on' because the user may forget about new postings in a subforum he has yet to read.

On the other hand, a user may choose not to read a subforum because he's not interested in the topics discussed there, so by reading the parent forum he sees that the subforum is new, doesn't read those but wants to have the parent forum marked as read anyway. This behaviour does not make sense in a news system but it may be necessary for forums, since users can not 'unsubscribe' from a subforum that they do not wish to read.

To implement this I have to refine my logic a little. Mark the forum as 'on' when:
- the lastpost timestamp > users forumsread timestamp
- or when a subforum is marked as 'on' AND the 'on' timestamp > users forumsread timestamp

It would be possible to make the behaviour optional, i.e. mark forums as 'on' if a subforum is 'on' and the mark as read marks all subforums as read too, or mark forums as 'on' if a subforum is 'on' with a newer timestamp, and do not mark subforums as read.

I'll implement this and see how it goes.

[frostschutz]

I decided to make my changes in a plugin. It's easier to handle than a patch (that windows users usually don't know how to apply), and if I make my idea available as a plugin, maybe it will attract some users to test it thoroughly just to see wether it can work properly at all or not.

---

Plugin thread is here: http://community.mybboard.net/thread-40381.html

---

The plugin version 0.3 now does by default nothing except fix the bug mentioned here. Advanced features (icons, sorting) are entirely optional and must be enabled first. I think that's as much effort as I'll put in this for now. If someone reports a bug regarding the plugin, I'll fix it. So far no one reported back but very few people downloaded the file in total. Plugin is still awaiting validation on the mods site.

[cHarOn]

thx that someone does an work on it i will try your plugin because i also dont like it how it is handled right now

[mybbfan19]

And it will be fix?