Colonel_mortis

One frequent complaint that I see is that every question is mandatory, which forces members to add a "not applicable" option when they have multiple questions. The poll title is a waste of time, given that there's already a topic title and poll question title I think it would be really neat if polls were attached to posts rather than to topics - that would force people to actually read the first post before answering the poll, and would allow for people to add polls in replies (and other places such as status updates) too

I've not really played around with achievements yet, but I think that's all the more reason to give this feedback now. My initial impression as a user on this site is that it's not obvious: What points are How I get points, and how many points things are worth How many points I have How the ranks compare (beyond just that each one is higher than the previous) (ie how many points each rank is worth) What I can do to unlock new badges (there's an argument that some badges should be secret, but certainly most should not) These are the sorts of things that experienced members will figure out within a couple of weeks, and maybe all that information is available for users somewhere that I'm not immediately seeing. However, the fact that I'm not immediately seeing it is the problem. For achievements to really work they need to be walk-up usable - a new member needs to be incentivized to ask questions, to post good answers, and to come back to the site next week, and right now, it doesn't do that. On the user menu, it highlights my rank and progress, which is great, but I want to click it and see more details. It feels weirdly non-interactive, and leaves me wanting more info. For a good case study, take a look at Stack Overflow. I can look at all the badges, and see my progress towards them. I also see a graph of my rep (which on SO is less valuable because rep is compounding as people find your old questions, but on forums would be a more active incentive to keep increasing it). Building on the SO example, wouldn't it be neat if achievement ranks could be configured to actually benefit you? Even the group promotion rules are conspicuously missing the ability to promote by rank. I want to set some rules like "level 1 can only use forums, level 2 can use blogs and status updates, level 3 can post news, level 4 can use classifieds, level 5 gets more messenger storage, etc", which I think would be a great way to incentivise engagement and give users a goal to work towards that is more than just a slightly different coloured icon on their profile picture. Also, and this is borderline a bug, the default badge descriptions are inconsistent - some don't have descriptions, some have passive descriptions ("a week since joining"), some have present tense descriptions ("making your first post"), and some have past tense descriptions ("visited daily for a week"). There's also no way to edit them (which I will report as a bug). Another thing - wouldn't it be nice if getting a badge could give you points? As far as I can see, that's not a thing at the moment.

When topics are merged, the view (manage) requests are redirected, but not any other controller methods, including not embeds. The redirects are really useful, but given that most links are actually in the form of embeds there's a significant gap there. There's also a lot of room for improvement on stopping people from destructively merging the wrong way on big topics (followers don't get ported across, and are irrecoverably deleted, maybe some other things too). The first post in the topic will always be the oldest, regardless of which was kept, and in almost every case I can think of, that means I would want to keep that post's title and therefore URL as well.

I had a few complaints about it from my members too, and I ended up fixing it by changing the logic for sending a new notification to not send it if it's from an IP that they have used previously (except moderators and admins, which I retain the old behaviour). Since making that change, I haven't had any complaints from members. if ( \IPS\Settings::i()->new_device_email && ($member->isAdmin() || $member->modPermission() || \IPS\Db::i()->select('count(*)', 'core_members_known_ip_addresses', ['member_id=? AND ip_address=?', $member->member_id, \IPS\Request::i()->ipAddress()] )->first() === 0) )

You forgot "Fixed a remote code execution vulnerability" (which has existed since at least 4.2).

I cheated, it doesn't do that by default.

@Colonel_mortis - feel free to step in if I am getting the wrong end of the stick here. Exactly But quotes that are replies to other members get discarded. The first thing that comes to mind is changing the overall look of quotes like the one above, essentially offering two types of quotes: A reply to someone An actual quote Maybe the #2. looks like: Am I finally understanding it lolol? Yeah, some sort of styling difference between the quote types might make sense. I think I'd want something that stands out slightly more than that, such as by adding a left border, such as stack overflow:

Nice suggestion, @Colonel_mortis Do you have any visual examples handy by chance? Something like this? I was just thinking they should look something like this ^ (but only with manually added quotes, not "real" quotes as in this example). Having pull quotes like in your example is also an interesting idea, and one that I could maybe see some sites using, although my use case is people quoting news articles, for which I don't think that would be super useful.

If I have a manually added quote (ie a quote added using the quote button rather than by quoting another post), it is often a relevant part of the actual post (especially when it's in the first post in a topic), so it would make sense for those quotes to remain as nested quotes when that post is quoted (especially now long quotes are truncated). It wouldn't be too hard to achieve - just look for the attribution attributes on the quote before stripping it.

I realise this ship has probably long since sailed, but we're 3 months into 4.5.4, and there's a decent pile of bugs, including a currently unpatched security related one, that have piled up in that time. Is there any chance we can have a 4.5.5 release to fix some of those bugs, rather than watching 4.6 stretch further and further out? I pushed for the features to be included in 4.6 rather than 4.5.5, but I did not intend for that to be at the expense of the timely release of other changes. (I also recognise wanting to give staff a break etc, but we're well into 2021 now, and surely having more bugs fixed would reduce their load anyway?)

Another one I've just run into - \IPS\Content\Comment::contentImages calls $item->container() indiscriminately, which results in a bad method call exception. This can be triggered when viewing an activity stream RSS feed.

When an ips.ui.editor instance is destroyed, you are following the answer from a Stack Overflow question and trying to manually destroy the CKE instance. Unfortunately, that manual destruction is not sufficient, and is leaving references to the editor and behaviour. Take ipsautosave as an example (and maybe the only example, I've not looked further) - it maintains a reference to `editor`, and runs autosave on it every two seconds. It attaches a `destroy` event handler, which would be triggered when calling editor.destroy(), to cancel the timeout and therefore release the editor reference, but because you never trigger the destroy event it's not sufficient. (I'm noticing this because I am trying to use multiple editors on a page with the same autosave key, but it's still a problem without that.) It looks like there are a few other references that destroy manually cleans up, so there are presumably a few other things that are leaked too. At a minimum, you should be doing `editor.fire("destroy")` to trigger all of that cleanup code, but ideally you should find the things that prevent editor.destroy() from working and fix those in CKE. As a middle ground, maybe you could do `try { editor.destroy(); } catch (e) { /* existing manual best effort */ }`.

Ugh it's the patch from ticket #70349 again. Looking at the patch again it doesn't seem to break either of the things that I've run into, so it seems I somehow managed to misapply it (my linux git doesn't like that patch file at all, it's missing the a/ and b/ prefixes). Apologies for wasting your time twice for that.

If you've not discovered it yet, I'm also getting TypeError: array_merge(): Argument #2 must be of type array, null given (0) in \IPS\forums\Forum::formatFormValues:1120 (neither $whereString nor $whereParams are defined as far as I and my IDE can tell, and $where seems to be unused). Also there are a few places where you have new\DateInterval( which seems to now be lexed as calling the function DateInterval in the namespace new, rather than applying the new operator to the root-namespace class DateInterval [1].

I've only been playing around with it IN_DEV so far, but I'm not noticing a difference. I don't think the JIT compiler is expected to make too much difference for web apps, but I would love to be wrong there.

PHP 8.0.0 has been released. Some bugs that I've found in a couple of minutes of testing: There are a bunch of deprecation warnings for required parameters following optional parameters - from my search with the regex function\s*\w+\s*\([^=)]*=[^)]*,\s*\$[^,)=]*[,)] there are 47 instances of this in the suite. It should be safe to remove all of the offending default parameters, since they can never be utilised. The cms lang key can_edit_item_message_record is invalid - it contains %S rather than %s, which causes the page to (randomly?) 500. Fatal error: Cannot make static method XMLReader::open() non static in class IPS\Xml\_XMLReader in .../system/Xml/XMLReader.php on line 34 (this might be a fun one to fix - it's a PHP 8 change that isn't compatible in either direction - https://www.php.net/manual/en/xmlreader.open.php#refsect1-xmlreader.open-changelog - you probably have to make a new method that delegates instead) TypeError: method_exists(): Argument #1 ($object_or_class) must be of type object|string, null given from statusContainer.phtml:133 (when viewing a feed containing status updates) PHP Fatal error: Unparenthesized `a ? b : c ? d : e` is not supported. Use either `(a ? b : c) ? d : e` or `a ? b : (c ? d : e)` in .../applications/calendar/widgets/upcomingEvents.php on line 153 If I come across any more issues, I'll add them in this topic, and encourage others to do the same.

It's also redundant when referring to a post within a topic - the title is displayed twice, with different formatting each time.

Of the 147 members who were classified as 4 but not FASed, about 30 are likely spam accounts (based on manual classification by one of my moderators), which gives a precision of 61% (39% false positive rate) when classifying based on a score of 4. I'd be happy to share some more detailed data with you if there's anything that you think would be helpful - feel free to reach out by ticket/email/PM(/slack), whatever is easiest.

Actually, looking further into the members who were caught by the spam service but weren't flagged as spammers, there are several who have made 0 posts (and thus weren't caught in my previous audit) but who are likely to be actual spammers based on their profile information. Based on the sample that I checked, the false positive rate is still too high to be useful, but it is not as high as I had originally thought.

I run a large site, so I have a lot of legitimate members and receive a fairly significant number of spammers. I turned it off (but continued to collect members' spam scores) several months ago, due to the volume of requests by legitimate people who were being caught by the spam filter. I've run some statistics based on registrations over the past month, comparing whether they have been flagged as spammer (which is very closely correlated with whether they were actually a spammer (this may not be true - see my post later in the topic)) and the code that the IPS spam service gave. The results are: +-------+------+----------+ | FASed | code | count | +-------+------+----------+ | 0 | 0 | 77 | | 0 | 1 | 5657 | | 0 | 2 | 3 | | 0 | 3 | 5 | | 0 | 4 | 147 | | 0 | null | 233 | | 1 | 0 | 3 | | 1 | 1 | 616 | | 1 | 2 | 13 | | 1 | 3 | 75 | | 1 | 4 | 155 | | 1 | null | 2 | +-------+------+----------+ Breaking this down: 12% of registrations are spammers 6% of registrations receive a spam score > 1 4% of registrations receive a spam score of 4 So far, these numbers don't seem unreasonable. However, If someone receives a spam score of 4 ("user is a known spammer"), they have 51% chance of actually being a spammer A precision of 51% is totally useless. If someone is actually a spammer, they have a 28% chance of receiving a spam score > 1. That's a pretty shoddy recall too. If I set my site to reject members with a spam score of 4, I will lose ~150 members to the spam filter each month; even if I use 2 as the threshold instead it will still only reduce spam by 28%. That's not an OK trade-off to me. You may say that the answer to this is to set accounts to require admin validation instead when caught by the spam filter, and that's not unreasonable. However, I'm not logged into ACP every day, so I suspect this would result in the loss of a large portion of the potential registrations who will just go and ask their question on a different forum because they didn't want to wait. Furthermore, it's often not possible to tell whether the account is legitimate just based on the registration info, so that 28% hit rate is going to drop quite a lot more. I appreciate that catching spam is a very hard problem. However, I believe these number demonstrate that the current system is not fit for purpose, at least with the level of confidence that you currently assign to it ("member is a known spammer" in the config page is a long way from the truth, and "certain spammer" from your marketing materials is an outright lie).

This was also brought up in the following topic, but there was no answer from IPS there and I do believe it is solvable: In almost all places where Item::$containerNodeClass is used, there is an isset test before accessing it and graceful fallback when it's not present. However, that check is missing in a few places that affect following and tagging. For followable: \IPS\core\Followed\Table:99 - this join should actually be within the preceding if statement (since I believe it only makes sense when permissions are used) core/front/table/tables/rows.phtml:104 - the if should also check that method_exists, as it the pattern everywhere else (but this isn't insurmountable because it will use the table desc if available) core/front/tables/manageFollowRow.phtml:43 - as above \IPS\Content\Search\Elastic\Query:467~487 - (what's going on with that code structure?) you should only honour $includeContainers if isset($class::$containerNodeClass) \IPS\Content\Search\Mysql\Query:416~436 - as above for tags: \IPS\Content\Item:7684 - should also check isset($containerClass) \IPS\Content\Item:7727 - I think this should be a constant if containerClass is empty, but I'm not sure how it's used so I may be wrong here. To my eye, all those changes should be feasible, and would make it much easier to avoid nasty hacks involving pseudo-nodes. nb. I searched for these occurences using intellij, so there may be places that I missed because the static analysis wasn't powerful enough.

That's a gmail-specific thing, although some other email providers do something similar. It would not be safe to blanket strip dots from emails, because that could open up security vulnerabilities relating to emails that do distinguish dots.

I have turned up the threshold for falling back to manual queries, because the vast majority of the time the queries that require manual action only take a couple of seconds to run (even with the threshold turned up). Maybe if I turn it down that will solve the issue, but instead it will mean I spend time copying queries into mysql-client instead. That raises another question though - can you fix the query runtime heuristic to be more accurate? UPDATE queries are pretty fast, as are DROP INDEX and I think DROP COLUMN, whereas CREATE INDEX queries are very slow, but they're all judged by the same threshold. Given the replies above though, as well as my memory of prior upgrades where I hadn't updated the threshold, resetting the threshold back to default is not the magic bullet here.

If one request times out in the JS upgrader, it falls back to the considerably slower HTML-based upgrader. Could there not be some logic to retry the request a couple of times, ideally with a bit of backoff (if it's an nginx or transport layer error it's probably a timeout, and retrying will then hit the case where the task is already done), rather than making the upgrade experience suck? At worst, the HTML upgrader shouldn't have delays between redirects - as soon as the response comes back it is ready to receive another request. It's reasonably easy to work around by just deleting the mr parameter from the URL, but that relies on me paying attention to the upgrader.

I don't say this much about this software, but the new ACP onboarding wizard is great. It has almost everything you need to bootstrap the community. However, it is missing settings to configure the ingoing and outgoing email addresses, and (less critically) the SMTP/etc settings, which means that emails default to being sent from the admin user's address (and therefore often don't get delivered).