Skip to main content
Topic: "New" Not Showing Properly (Read 6597 times) previous topic - next topic
0 Members and 1 Guest are viewing this topic.

"New" Not Showing Properly

On this topic: http://www.elkarte.net/index.php?topic=443.30

It doesn't not show me the "new" tag beside any topics in the MessageIndex, but the BoardIndex it is in(Site Feedback) shows as bold(as in new topic/reply).

Other topic in other boards have the "new" label if there is a new topic/reply.

Re: "New" Not Showing Properly

Reply #1

Yep, noticed, but I didn't have time to debug it (I was breaking other things :P).

I think it is the first time you enter a board (i.e. if you have two boards with unread messages, the first you enter doesn't give the "new" message, while the others do), but I'm not 100% sure.
If it is so, it may be related somehow to the session.
Bugs creator.
Features destroyer.
Template killer.

 

Re: "New" Not Showing Properly

Reply #2

I haven't been as involved with the community since this issue. It seems this issue is still there. I don't visit everyday so I can't tell which posts are new per topic or not.

Re: "New" Not Showing Properly

Reply #3

Yep, still there.
I think (not sure though) it has been solved in the repo, but the site is kind of outdated... O:-)
Bugs creator.
Features destroyer.
Template killer.

Re: "New" Not Showing Properly

Reply #4

Seems to be fixed now. Thanks.

Re: "New" Not Showing Properly

Reply #5

It's fixed on the site, but is still broken in the code.
The issue is in the  Request class, where $_REQUEST['start'] is always considered an integer, while in few places it is not.
Bugs creator.
Features destroyer.
Template killer.

Re: "New" Not Showing Properly

Reply #6

If you want I'll make a PR for that ... but basically I removed that limitation vs change all the code to enforce that limitation.  Otherwise we can wait for Norv to have some free time since that was her code change and she should know best which direction.

Re: "New" Not Showing Properly

Reply #7

I don't know exactly where put my hands to fix it "properly", mostly because there are at least two "exceptions" (to the (int) rule) and both are slightly different as far as I've seen, so they would require an ad hoc implementation of the cleanup.

If the do-not-cast-to-int is enough for now, I can push it to the branch I have open or if you want feel free to push it in your PR! :P

 emanuele likes the second option because is less work for him. :P
Bugs creator.
Features destroyer.
Template killer.

Re: "New" Not Showing Properly

Reply #8

Feel free to make the PR :) ... for now I'd go with the do not cast solution since as you noted, the other fixes are quite a bit more involved and lets face it, its been un-cast for a heck of a long time  O:-)  (Does not mean its proper, just that it does not seem harmful)


Re: "New" Not Showing Properly

Reply #10

Does it need to be set to zero if its not set ?  What I did on the site was:
Code: [Select]
$_REQUEST['start'] = isset($_REQUEST['start']) ? $_REQUEST['start'] : 0;


Re: "New" Not Showing Properly

Reply #11

There is a check (luckily, I didn't see it at first :P) at line 262 that seems to take care of that. O:-)
Bugs creator.
Features destroyer.
Template killer.

Re: "New" Not Showing Properly

Reply #12

 8)

Re: "New" Not Showing Properly

Reply #13

Maybe you could have a little regex for that. I don't remember the exceptions but something like this:

Code: [Select]
$_REQUEST['start'] = isset($_REQUEST['start']) && preg_match('~^(:?(:?from|msg)?\d+|new)$~', $_REQUEST['start']) ?  $_REQUEST['start'] : 0;

A little test code too, if you can think of more cases:

Code: [Select]
<?php

$regex = '~^(:?(:?from|msg)?\d+|new)$~';

$tests = array(
 '123' => 1,
 'new' => 1,
 'msg123' => 1,
 'from123' => 1,
 '123msg' => 0,
 'from12x3' => 0,
 'new123' => 0,
 'news' => 0,
 'msg1.23' => 0,
);


echo '
<style>
table
{
 border-collapse:collapse;
}
td, th
{
 border: 1px solid #000;
 padding: 0.5em;
}
</style>
<table>
 <tr>
 <th>Test</th>
 <th>Expected</th>
 <th>Result</th>
 </tr>';

foreach ($tests as $test => $expected)
{
 $result = preg_match($regex, $test);
 echo '
 <tr>
 <td>', $test, '</td>
 <td>', $expected, '</td>
 <td>', $result, '</td>
 </tr>';
}

echo '
</table>';

Reminds me of the time we were coding compareVersions() and matchPackageVersion() functions right before 2.0 release with Norv and came up with 100+ cases to test. Good old days...

Quote from: emanuele – There is a check (luckily, I didn't see it at first :P) at line 262 that seems to take care of that. O:-)

That turned out to be a pretty weird check but interestingly it actually prevented a PHP bug to be exploited.

Re: "New" Not Showing Properly

Reply #14

Thanks [SiNaN]!
Will try for sure.
I don't know exactly all the exceptions me either, but if we find some we can add them to the list. :P

Quote from: [SiNaN] – That turned out to be a pretty weird check but interestingly it actually prevented a PHP bug to be exploited.
Norv wrote it, you can't expect anything less than weird checks. :P
Bugs creator.
Features destroyer.
Template killer.