MyBB Community Forums

And there we are, back to the beginning of the circle.

You are already recursing through the forums... that's exactly what the build_forumbits function is doing, recurse through forums. It's a recursive function by design. Recursion is already done to find the lastpost of a forum or its subforums. What my patch does is use the information obtained in this recursion a little differently.

And I don't think it's fair to nix this just because some very big forum chooses to use quadrillions of subforums. This issue is an annoyance to all forums, especially the smaller ones that don't have thousands of users online at any time of the day. In a forum where not all bulbs are always on anyway because someone always posts something new somewhere, it's all the more important to have a proper lightbulb logic.

You can shy away from this problem forever, other forum systems don't and they even go a step further by offering combined status icons that show current forum and subforum status in one image (which is what I also do in my plugin). I do not know wether other forums implementation is perfect and I also do not know wether my own implementation is perfect, but I do know that the current implementation of MyBB is just wrong, blatantly obviously so.

There are a lot of very very fricken huge MyBB forums.

Two solutions:

1) Change to SM(er)F;
2) Add the functionality in that automatically enables up until a point that it will overload the server, then it just switches back to the 'legacy' way.

(2009-01-30, 11:54 AM)bobbit Wrote: [ -> ]Add the functionality in that automatically enables up until a point that it will overload the server, then it just switches back to the 'legacy' way.

Erm, that's not very smart - First of all how can we tell what is "overload" or not? Second, that just makes it extremely inconsistent and confusing to the user.

(2009-01-30, 02:29 AM)frostschutz Wrote: [ -> ]And there we are, back to the beginning of the circle.

You are already recursing through the forums... that's exactly what the build_forumbits function is doing, recurse through forums. It's a recursive function by design. Recursion is already done to find the lastpost of a forum or its subforums. What my patch does is use the information obtained in this recursion a little differently.

And I don't think it's fair to nix this just because some very big forum chooses to use quadrillions of subforums. This issue is an annoyance to all forums, especially the smaller ones that don't have thousands of users online at any time of the day. In a forum where not all bulbs are always on anyway because someone always posts something new somewhere, it's all the more important to have a proper lightbulb logic.

You can shy away from this problem forever, other forum systems don't and they even go a step further by offering combined status icons that show current forum and subforum status in one image (which is what I also do in my plugin). I do not know wether other forums implementation is perfect and I also do not know wether my own implementation is perfect, but I do know that the current implementation of MyBB is just wrong, blatantly obviously so.

As for you frostschutz, if you had spent any time actually testing your code versus just complaining about ours would have noticed several things.

I just tested your patch on my localhost. Well let's just say it completely breaks the the status indicators. As a guest, I navigated into a forum with unread posts and without touching any of the unread threads, I simply navigated back to the index and now the forum was marked as "read". This was WITHOUT reading any threads so clearly the forum should of still been marked as unread.

Even worse, as a logging in user on my test forum, The subforum which has 1 unread thread is marked as read. It never ever displayed the unread icon. If I implemented that fix that would be a step backwards for MyBB.

So yeh, maybe it would do you some good to test your fixes before you go post them here and then flaunt that we're oh-so-bad and we do nothing to improve our product.


Furthermore, the solution has to lay in the places where we insert into the forumsread table. That would be when we view a thread it needs to check the threadsread table to make sure we've read the all the threads in that forum. From there it inserts a record based on that.

For example we had the following forum structure:
- Category
-- Forum 1
--- Subforum 1
--- Subforum 2
----Subforum subforum 1
-- Forum 2
-- Forum 3
--- Subforum 3
---- Subforum subforum 4

Now, lets say we just finished reading the entire Subforum 2 forum and all the rest are already read except for Forum 2. If you wanted it to be accurate to do that for every single child and every single parent then we'd have to literally count all the threads from the Subforum 2 and make sure that the same number of threads are in the threadsread table. Then we'd have to do it again for Forum 1. Then we'd have to do it again for the Category. And it just gets more and more intensive on forums with a larger then average directory tree.

(2009-01-30, 02:47 PM)Ryan Gordon Wrote: [ -> ]As a guest, I navigated into a forum with unread posts and without touching any of the unread threads, I simply navigated back to the index and now the forum was marked as "read". This was WITHOUT reading any threads so clearly the forum should of still been marked as unread.

That's original MyBB behaviour for guests.

Quote:Even worse, as a logging in user on my test forum, The subforum which has 1 unread thread is marked as read. It never ever displayed the unread icon. If I implemented that fix that would be a step backwards for MyBB.

That, too, happens to me with original MyBB.

For me the bulbs are correct as long as I stay logged in with one user. When logging out, and then logging back in, the user lastread timestamps somehow get messed up. However this is what happens with original MyBB for me. In my patch, I do not change anything about how or when MyBB updates / sets / stores the timestamps.

All my patch does, is actually look at the timestamps that are already there. It assumes that the timestamps are already correct for each forum. The original MyBB logic simply ignores them all and just looks at the lastpost of an entirely different (sub)forum, which will never work.

I tested my patch a lot, and couldn't find issues related to my patch. Of course there may be bugs in it that I have overlooked nevertheless. However the issues you listed seem to be different issues located in code that my patch does not touch.

---

Regarding the forumsread table, for each forum it must be known wether that forum is read or not. That should be fine as it is now, it's just a timestamp per forum and user. If that timestamp is newer than lastpost of the forum, then the forum is read. Otherwise there are unread threads in that forum. It's simple enough.

Forumsread table does not have to concern itself with tree or subforum structure at all. build_forumbits does the traversing and figuring out, when a parent forum should be marked unread due to a subforum being unread. It's just one single loop over the list of forums, or would be if it wasn't recursive. And that's what build_forumbits is already doing - not very efficiently so even.

What doesn't work with this simple model?

---

The (not necessarily root) cause for the forum appearing as read even though it has read posts is this code in get_forum_lightbulb (original):

                // 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'];
                }

For debugging, I added a $origin = "forum", $origin = "cookie", $origin = "visit" in each of these cases, and added the $origin below into the alt (altonoff) string of the bulb (which usually just said 'forum contains new posts' on mouseover).

The result is very inconsistent. For some forums it gets the timestamp from the $forum variable, for others it uses the cookie, for even others it uses the visit date. And those visit timestamps are what is causing the forum to show as read, even though it's really unread. Your lastvisit seems to be just when you logged in, so posts that were written before do not show.

So the fix in this case would be to make sure that the forum var always holds the correct data. Not sure why this is not already the case, could have something to do with new users / empty forums, can't seem to reproduce it with an older user / more filled forum.

---

Guests use 'visit' a lot, but once they browse into a forum and see unread threads there and go back without reading them, it switches to 'forum' and marks all the threads in the forum as read (original MyBB).

---

My guess is that instead of storing the lastvisit for a new guest / user on the very first visit, it keeps on using lastvisit forever until the first time the new guest / user consciencly has read the forum. So the user has either to click on, and read every forum, or click on mark all as read once.

So when you logout / login, the last visit changes, and suddenly the forums that were showed as unread before due to an older last visit, show up as read then because of the new lastvisit.

Instead of using lastvisit, it should use lastvisit minus some time threshold (wasn't there already a bug report related to this?) AND store that timestamp like it stores it when actually reading the forum, so the timestamp based on lastvisit will actually only be used on the very first visit / login to a forum and not on visits / login that happen at a later time.

(2009-01-30, 03:34 PM)frostschutz Wrote: [ -> ]

(2009-01-30, 02:47 PM)Ryan Gordon Wrote: [ -> ]As a guest, I navigated into a forum with unread posts and without touching any of the unread threads, I simply navigated back to the index and now the forum was marked as "read". This was WITHOUT reading any threads so clearly the forum should of still been marked as unread.

That's original MyBB behaviour for guests.

Quote:Even worse, as a logging in user on my test forum, The subforum which has 1 unread thread is marked as read. It never ever displayed the unread icon. If I implemented that fix that would be a step backwards for MyBB.

That, too, happens to me with original MyBB.

Hm, your right. I was rushing out to school so I didn't double check. However, I definitely didn't see the patch improve anything.

(2009-01-30, 03:34 PM)frostschutz Wrote: [ -> ]Regarding the forumsread table, for each forum it must be known wether that forum is read or not. That should be fine as it is now, it's just a timestamp per forum and user. If that timestamp is newer than lastpost of the forum, then the forum is read. Otherwise there are unread threads in that forum. It's simple enough.

Correct, but since we don't traverse completely through the forum array tree and insert a record for each forum, forums that should be read aren't marked as so in the forumsread table. While we may try to workaround with different logic in the build forum bits, it's complex to do so properly.

(2009-01-30, 03:34 PM)frostschutz Wrote: [ -> ]The (not necessarily root) cause for the forum appearing as read even though it has read posts is this code in get_forum_lightbulb (original):

                // 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'];
                }

For debugging, I added a $origin = "forum", $origin = "cookie", $origin = "visit" in each of these cases, and added the $origin below into the alt (altonoff) string of the bulb (which usually just said 'forum contains new posts' on mouseover).

Very interesting. visit should only be used as a fall back. lastread should be set if the forums has been read, regardless of being logged in or not.

if($mybb->user['uid'] == 0)
{
	// Build a forum cache.
	$query = $db->query("
		SELECT *
		FROM ".TABLE_PREFIX."forums
		WHERE active != 0
		ORDER BY pid, disporder
	");
	
	$forumsread = unserialize($mybb->cookies['mybb']['forumread']);
}
else
{
	// Build a forum cache.
	$query = $db->query("
		SELECT f.*, fr.dateline AS lastread
		FROM ".TABLE_PREFIX."forums f
		LEFT JOIN ".TABLE_PREFIX."forumsread fr ON (fr.fid=f.fid AND fr.uid='{$mybb->user['uid']}')
		WHERE f.active != 0
		ORDER BY pid, disporder
	");
}

while($forum = $db->fetch_array($query))
{
	if($mybb->user['uid'] == 0)
	{
		if($forumsread[$forum['fid']])
		{
			$forum['lastread'] = $forumsread[$forum['fid']];
		}
	}
	$fcache[$forum['pid']][$forum['disporder']][$forum['fid']] = $forum;
}

So that is interesting that it falls back on other incorrect dates. I don't think that should happen. What happens if you just change that faulty bit of code to:

                // Fetch the last read date for this forum
                if($forum['lastread'])
                {
                        $forum_read = $forum['lastread'];
                }
                else
                {
                        $forum_read = 0;
                }

I haven't read into the timestamps that deeply before, I just assumed they were always correct Which they usually seem to be, at least for registered users that have read all forums at least once in their lifetime.

With your change, all forums show up as unread for new users. And once you read the forum, well, there is a proper timestamp for it then. I haven't tested yet what happens with threadreadcut etc, wether the forum would show up as unread, and then as read once you click on it, without actually reading anything, because the threads were too old.

Now that you pointed me to these timestamp issues, I'm afraid that there are several oddities related to timestamps that need fixing elsewhere. The way it's done currently doesn't seem to be right for guests and new users, and just setting it to 0 may cause issues elsewhere.

So for a 100% consistent handling, both timestamps and buildforumbits would have to be fixed.

I'll look into timestamps, but it'll take some time. Unless you want to investigate yourself of course

I'll be investigating myself as well, I was just at school at the time that I posted that message so I didn't have a way to test it.

Okay so this:

if(!$forum_read)
{
       $forum_read = $mybb->user['lastvisit'];
}

is our little hack/workaround for the "Mark all forums as Read" for guests. Since cookies have a limit, sometimes it would be impossible to properly store all the forum ids in the forumsread cookie.

Okay so I think I found a few issues. First is in inc/functions_indicators.php.

function mark_thread_read($tid, $fid)
{
	global $mybb, $db;

	// Can only do "true" tracking for registered users
	if($mybb->settings['threadreadcut'] > 0 && $mybb->user['uid'])
	{
		// For registered users, store the information in the database.
		switch($db->type)
		{
			case "pgsql":
				$db->shutdown_query($db->build_replace_query("threadsread", array('tid' => $tid, 'uid' => $mybb->user['uid'], 'dateline' => TIME_NOW), "tid"));
				break;
			default:
				$db->write_query("
					REPLACE INTO ".TABLE_PREFIX."threadsread (tid, uid, dateline)
					VALUES('$tid', '{$mybb->user['uid']}', '".TIME_NOW."')
				");
		}

		// Fetch ALL of the child forums of this forum
		$forums = get_child_list($fid);
		$forums[] = $fid;
		$forums = implode(",", $forums);

		$unread_count = fetch_unread_count($forums);
		if($unread_count == 0)
		{
			mark_forum_read($fid);
		}
	}
	// Default back to cookie marking
	else
	{
		my_set_array_cookie("threadread", $tid, TIME_NOW);
	}
}

should probably be:

function mark_thread_read($tid, $fid)
{
	global $mybb, $db;

	// Can only do "true" tracking for registered users
	if($mybb->settings['threadreadcut'] > 0 && $mybb->user['uid'])
	{
		// For registered users, store the information in the database.
		switch($db->type)
		{
			case "pgsql":
				$db->shutdown_query($db->build_replace_query("threadsread", array('tid' => $tid, 'uid' => $mybb->user['uid'], 'dateline' => TIME_NOW), "tid"));
				break;
			default:
				$db->write_query("
					REPLACE INTO ".TABLE_PREFIX."threadsread (tid, uid, dateline)
					VALUES('$tid', '{$mybb->user['uid']}', '".TIME_NOW."')
				");
		}
	}
	// Default back to cookie marking
	else
	{
		my_set_array_cookie("threadread", $tid, TIME_NOW);
	}

	$unread_count = fetch_unread_count($fid);
	if($unread_count == 0)
	{
		mark_forum_read($fid);
	}
}

Because we should still be performing the mark_forum_read check for guests as well.

And then in debugging that functionality, when an ajax reply is made and all the threads are already read in that forum and in the sub forums, it's still marked as unread. So I tracked it down to the my_set_array_cookie in inc/functions.php

function my_set_array_cookie($name, $id, $value)
{
	global $mybb;
	
	$cookie = $mybb->cookies['mybb'];
	$newcookie = unserialize($cookie[$name]);
	$newcookie[$id] = $value;
	$newcookie = addslashes(serialize($newcookie));
	my_setcookie("mybb[$name]", $newcookie);
}

should be

function my_set_array_cookie($name, $id, $value)
{
	global $mybb;
	
	$cookie = $mybb->cookies['mybb'];
	$newcookie = unserialize($cookie[$name]);
	$newcookie[$id] = $value;
	$newcookie = serialize($newcookie);
	my_setcookie("mybb[$name]", addslashes($newcookie));
	
	// Make sure our current viarables are up-to-date as well
	$mybb->cookies['mybb'][$name] = $newcookie;
}

And that's because, when we go to ajaxly insert the thread and update the read threads cookie array, all in the same go, we then run the fetch_unread_count($fid) function. As a guest it uses the internal MyBB threadsread cookie array to figure out whether or not we have unread forums or not. Since the $mybb->cookies array wasn't being updated after the change was actually made it was out of date.

So that caused that issue.

Okay found 1 more thing preventing the guest ajax thingy from updating. in newreply.php find:

// Mark thread as read
require_once MYBB_ROOT."inc/functions_indicators.php";
mark_thread_read($tid, $fid);

and move that bit of code to right behind:

// Check captcha image
if($mybb->settings['captchaimage'] == 1 && function_exists("imagepng") && !$mybb->user['uid'])
{

I also may have an experimental fix. It doesn't change too terribly much, but it's getting a bit late here and my brain is slowing down from the exhaustive debugging so we'll see how badly this patch screws up the indicators


So once you've applied all the above fixes in my previous post and the one in this, download the attached file and overwrite it in the inc/ directory.