Nao

  • Dadman with a boy
  • Posts: 16,079
Couple things to look into...
« on March 18th, 2017, 11:25 AM »
I really, really don't have time these days, so can anyone (okay, CerealGuy) look into these potential issues?

- DoLogin() in Subs-Login accesses we::$user['ip'] (previously $user_info['ip']). I couldn't find any reference to we::$user being set during the login process, leading me to believe maybe we should use $user_settings everywhere across that short function. Can you find one reference to it..?

- On LT I've been marking topics as unread and noticed the number of unread counts was higher than before I reached said topic. I specifically rewrote the unread system to make sure it would keep track of it. Can you look into it and determine if I screwed up something..? :edit: Just tried on your last post on the BBC topic, and voilà, 9 unread posts... I definitely screwed something up then, because I'm positive that it used to work.

Thanks!

CerealGuy

  • Posts: 343
Re: Couple things to look into...
« Reply #1, on March 18th, 2017, 08:20 PM »
Will definetly look into the login thing, unread topics also but less priority.
There are some things which bug me about login (besides sha256):
- no js counter for failed logins, i find myself quite often on the "you have to wait before you can try to login again" error page.
- after x failed logins this error page should first show up, for me it shows up after the first try
- no redirect of $_POST content, i often loose posts because my session timed out. Don't know if this should get solved on login or in editor. Have to think about that.
- notification on x failed logins for the user would be cool, maybe plugin stuff.

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: Couple things to look into...
« Reply #2, on March 19th, 2017, 12:29 AM »
Quote from CerealGuy on March 18th, 2017, 08:20 PM
Will definetly look into the login thing, unread topics also but less priority.
Thanks!

Also, looks like your subs-bbc commit is broken... At the very least, 'list' or 'cli' tags aren't working properly (see New Revs topic). Dunno if it's a problem during the rewrite for 'list', or a problem with external custom bbcode not loading at all.
Quote
There are some things which bug me about login (besides sha256):
- no js counter for failed logins, i find myself quite often on the "you have to wait before you can try to login again" error page.
I don't know, TBH with LT I only use Steam Connect (i.e. OpenID) to handle users, so no one has to enter passwords on the site. So I don't really have a reason to look into this ahah.
Quote
- after x failed logins this error page should first show up, for me it shows up after the first try
Ah, really?
Quote
- no redirect of $_POST content, i often loose posts because my session timed out. Don't know if this should get solved on login or in editor. Have to think about that.
I thought $_POST got kept, actually...?
Quote
- notification on x failed logins for the user would be cool, maybe plugin stuff.
Yeah, more likely.
Re: Couple things to look into...
« Reply #3, on March 19th, 2017, 11:41 AM »Last edited on March 19th, 2017, 12:14 PM
Quote from Nao on March 19th, 2017, 12:29 AM
Also, looks like your subs-bbc commit is broken... At the very least, 'list' or 'cli' tags aren't working properly (see New Revs topic). Dunno if it's a problem during the rewrite for 'list', or a problem with external custom bbcode not loading at all.
Actually that's because the cli tag didn't have a plugin ID and you're only loading those.
This might be a problem if someone built a mod (rather than a plugin) that still added stuff to the database, but it's unlikely of course.

(This still isn't working yet, ahah.)

:edit: Was due to the fact that the list tag didn't have 'cli' in its allowed children.

CerealGuy

  • Posts: 343
Re: Couple things to look into...
« Reply #4, on March 19th, 2017, 03:25 PM »
Quote from Nao on March 19th, 2017, 11:41 AM
Actually that's because the cli tag didn't have a plugin ID and you're only loading those.
This might be a problem if someone built a mod (rather than a plugin) that still added stuff to the database, but it's unlikely of course.

(This still isn't working yet, ahah.)

 :edit: Was due to the fact that the list tag didn't have 'cli' in its allowed children.
This where part will get removed as soon as the default bbcodes got removed from the database. So only a temporary thing.
Re: Couple things to look into...
« Reply #5, on March 20th, 2017, 04:44 PM »
Quote from Nao on March 20th, 2017, 04:28 PM
[Commit revision 7fdc0bd]
Author: Nao
Date: Mon, 20 Mar 2017 16:28:03 +0100
Stats: 2 files changed; +6 (insertions), -6 (deletions)

[cli=f]This is a follow-up to rev c92eb3fb73b94a545029e73a830540e5779dfd5c from October 2013. I'd changed the Mark Topic as Unread system to better accomodate for infinite scrolling, but it also broke 'regular' use of the feature by resetting the read post counter to the beginning of the page, instead of the last unread post. I've tweaked the files to always reset to the last unread post, unless (1) you're on a read page (in which case it will mark the LAST post in THAT page as unread), (2) you're in infinite scrolling mode and you just viewed a new page (here it'll simply mark the penultimate read page's last post as unread.) This sounds complicated, but it works better for me. Until, of course, someone tells me it's broken... (Display.php, Subs-Boards.php)[/cli][/q]
Did this solve the unread thing?

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: Couple things to look into...
« Reply #6, on March 20th, 2017, 10:25 PM »
Well, I wouldn't have committed otherwise... ;)
But it's still in the air. I mean the problem with post manipulation inside topics is that there are so many niche situations... Like people showing topics in reverse order, etc... I'm just hoping nothing was broken.

CerealGuy

  • Posts: 343
Re: Couple things to look into...
« Reply #7, on March 30th, 2017, 01:29 PM »
And even some more:
- Mentions is sometimes not working. This is in my opinion something we could implement directly into wedge.


Nao

  • Dadman with a boy
  • Posts: 16,079
Re: Couple things to look into...
« Reply #8, on April 3rd, 2017, 10:53 PM »
I don't know about a direct implementation; I'm not against it, but it also serves as a good generic plugin example.
Also, it's never a perfect plugin. For instance, it only shows name suggestions when writing in a dedicated post page, and if it does show one, it's pretty hard to get rid of it. Oh, and multi-word names usually require editing the bbcode directly...
Re: Couple things to look into...
« Reply #9, on April 12th, 2017, 12:47 PM »
Did someone look into the Subs-Login semi-mystery?

I also have a quick note for optimization purposes. Just can't bother to add it to Wedge, but this should probably be in the next database update batch (remember? ;))...

I found out that {db_prefix}thoughts has no index on the id_parent column. It's often used in inner joins, so it should REALLY be there. I don't use thoughts a lot, so it's no biggie, but I can imagine that after a while, it'd have an influence over performance.