Colonel_mortis

Firefox, Linux When this notification was sent, there was only one new unread post in the topic, and I had no prior unread notifications (as judged by in-platform notifications). I'm a bit confused about where 6 has come from - it is less than the number of other members' replies to this topic (which I have been following since creation), but more than the number of replies since I last rebooted this computer. I usually don't clear my system notifications (the system notification list that these push notifications are delivered to), but I had manually cleared them prior to receiving this to see if that makes any difference. It didn't (or at least it didn't solve it, perhaps it would otherwise have come up with a different number?). I'm pretty sure this is something to do with the self.registration.getNotifications and unseenCount stuff in serviceWorker.js, but I'm not quite sure the semantics here (I assumed that clearing the notification list, or restarting my computer, would close all the open notifications). The correct behaviour here, in my mind, is that the numbers here should be no greater than the in-platform notification list - if there have been 3 notifications about replies to a given topic since I last read my in-platform notifications, the number that is shown here should represent the number of notifications about replies that are unread in both the in-platform notifications and push notifications (at most 3, but could be lower if I dismissed some native notifications). There are definitely alternative approaches that would make sense too, but it's pretty clear to me that the current approach is not correct. Maybe this is just a Firefox+Linux issue, I haven't tried reproing on any other platforms yet.

You can filter the table by banned (and validating, admin, etc), but not non-banned. This would be really useful when searching for spammers/bad users that match a certain pattern, where most of them have already been caught but you're looking for the ones that have slipped through the gaps.

To reproduce: (Prerequisites: lazy loading is enabled (on by default), and the default disk storage location is used for attachments) Attach an image to a post Look at that post in the DB - it should look something like <img class="ipsImage ipsImage_thumbnailed" data-fileid="1" data-ratio="75.00" width="640" alt="foo.jpg" data-src="<fileStore.core_Attachment>/monthly_2022_10/foo.jpg" src="<___base_url___>/applications/core/interface/js/spacer.png" /> Edit the post (don't need to change anything) Look at the post again in the DB - it should now look something like <img alt="foo.jpg" class="ipsImage ipsImage_thumbnailed" data-fileid="1" data-ratio="75.00" width="640" data-src="<___base_url___>/uploads/monthly_2022_10/foo.jpg" src="<___base_url___>/applications/core/interface/js/spacer.png" /> Notice that the data-src has changed from <fileStore.core_Attachment>/monthly_2022_10/foo.jpg to <___base_url___>/uploads/monthly_2022_10/foo.jpg, so it is no longer agnostic of the backing file store Moving the attachments filestore will now cause these images to point to the wrong place. This is because in \IPS\Text\Parser::_parseImgElement, you have /* Lazy-loaded? */ $srcAttribute = ( $element->hasAttribute( 'src' ) AND $element->hasAttribute( 'data-src' ) ) ? 'data-src' : 'src'; // ... /* Or an attachment? */ elseif ( $attachment = static::_getAttachment( $element->getAttribute($srcAttribute ), $element->hasAttribute('data-fileid') ? $element->getAttribute('data-fileid') : NULL ) ) { $file = $this->_getFile( 'core_Attachment', $element->getAttribute( $srcAttribute ) ); $element->setAttribute( 'data-fileid', $attachment['attach_id'] ); $element->setAttribute( 'src', str_replace( array( 'http:', 'https:' ), '', str_replace( static::$fileObjectClasses['core_Attachment']->baseUrl(), '{fileStore.core_Attachment}', $element->getAttribute( $srcAttribute ) ) ) ); the last line there is setting the 'src' attribute, when in fact it should be setting $srcAttribute. The src attribute is subsequently discarded to replace with the lazy load placeholder. This works for new posts because $srcAttribute is 'src' there, but when editing posts (with lazy loading enabled) it is actually data-src.

I have some gallery images that are failing to load with even on the default theme. In the DB the image_notes column is serialized using PHP serialization rather than JSON: a:1:{i:0;a:6:{s:2:"id";s:32:"f8ea194da97843715f1a41ceae34e3a9";s:3:"top";i:343;s:4:"left";i:76;s:5:"width";i:52;s:6:"height";i:52;s:4:"note";s:42:"Crucial M55 and 8GB of Corsair Vengeance. ";}} which throws off the deserialization in \IPS\gallery\Image::get__notes and subsequently the \count( $image->_notes ) in imageFrame.phtml. This seems to be the case for all of the images with an image_date value prior to Jan 2016 (when we upgraded from IPB 3.x to IPS 4.x) that have a non-empty image_notes column. I am fixing them manually for my site, although it's definitely not a high priority issue because it has presumably been an issue for years.

\IPS\nexus\Package, L2349, you have a comment saying However, if this case does get hit, there is also a risk that this user's page gets cached, so the user's cart gets shown to other guests, which would be kinda bad. Also, theoretically they won't be able to check out because the cart page may also be cached as empty or incorrect, though that adds additional constraints so is slightly less likely. Given that this hasn't been hit throughout the whole life of guest page caching being an actual feature, it's probably not super critical - it's probably super rare that someone adds stuff to their cart, then browses the site for 20 mins+ without clearing it - but I don't see any downside to setting the cookie for the session lifetime, to be consistent with everywhere else, and this does matter when caching is pushed to the CDN.

I have my own hooks to add additional spam filter rules, and I imagine I'm not alone. I want them to be able to take advantage of showing the reason that the post got mod queued, but there's no easy way to do that, because the post ID is only available after it's saved, and there's no suitable hook point in \IPS\Content::checkProfanityFilters between saving and constructing the approval entry. It would make me much happier if you replaced the switch statement on line 773 with either: A method call that we can hook Or fall back to setting $log->held_data = ["match" => $filtersMatched["match"]] Or just always do that, and drop the logic to choose between the cases entirely You already made Approval itself hookable by pulling availableReasons out into a method (though why it's necessary to have it at all is a bit confusing to me), so this feels like an unfortunate oversight. I believe the image hold log in \IPS\Content::shouldTriggerProfanityFilters on line 639-643 is also going to hit the issue with the post ID not being available yet, but I obviously don't pay enough to be able to test that. The changes suggested here should make that code easier to unify with the rest of the cases.

To reproduce: Add a word to the banned words filter, such as "spam" Make a new topic, including "spam" in the initial post Observe that the topic got added to the approval queue as expected Observe that there is no message saying that it was queued because it contains "spam" This is because for the first post in topics the approval entry is saved based on the topic, not the post itself, but there's no logic in a topic to render that information (and hiddenBlurb only loads the post itself).

To reproduce: Go to the full search page, /search/ Apply a date filter (either created or updated), with value set to "Custom" and a start and/or end date Enter a search term, and submit Refresh the page (or click one of the "Didn't find what you were looking for" suggested filters) Expand the search filter options again, and observe that the supplied date filter has been cleared Edit the search term, then submit the search request again, and observe that the date filter is no longer being applied This is because in core/html/front/search/filters.phtml, lines 193/197/219/223, you're directly stringifying the current value, which is a \IPS\DateTime {$elements['search_tab_content']['updatedDateCustom']->value['end']} which results in a non-ISO8601-compliant timestamp like "11/03/22 12:00 AM" (rather than 2022-11-03). You probably need to do something like you already have in the regular Date form element template value="{{if $value instanceof \IPS\DateTime}}{$value->format('Y-m-d')}{{else}}{$value}{{endif}}" (This was reported to me by a user using Firefox, and reproed also in Firefox. It's possible that other browsers tolerate the invalid values, I haven't checked, but doing so would not be standards compliant).

To reproduce: Create a moderator with the permissions set to "Restricted", but leave everything checked (the thing that actually matters here is that the "All" checkbox is enabled in the forum selector in the forums tab) (From any user) attempt to report a forum post Observe that you get redirected to an error saying "You have already reported that.", and that the original request returned 500 Stack trace: TypeError: in_array(): Argument #2 ($haystack) must be of type array, int given (0) #0 /opt/forum/system/Content/Content.php(1714): in_array() #1 /opt/forum/init.php(940) : eval()'d code(62): IPS\_Content->report() #2 /opt/forum/system/Content/Controller.php(3001): IPS\moderation_hook_Content->report() #3 /opt/forum/system/Content/Controller.php(2271): IPS\Content\_Controller->_report() #4 /opt/forum/applications/forums/modules/front/forums/topic.php(1240): IPS\Content\_Controller->__call() #5 /opt/forum/system/Dispatcher/Controller.php(107): IPS\forums\modules\front\forums\_topic->__call() #6 /opt/forum/system/Content/Controller.php(50): IPS\Dispatcher\_Controller->execute() #7 /opt/forum/applications/forums/modules/front/forums/topic.php(39): IPS\Content\_Controller->execute() #8 /opt/forum/system/Dispatcher/Dispatcher.php(153): IPS\forums\modules\front\forums\_topic->execute() #9 /opt/forum/index.php(13): IPS\_Dispatcher->run() #10 {main} The buggy code is \IPS\Content::report, L1710-1719: if ( $canView === TRUE and $container = $item->containerWrapper() and isset( $container::$modPerm ) ) { if ( isset( $perms[ $container::$modPerm ] ) and $perms[ $container::$modPerm ] != '*' ) { if ( ! in_array( $item->mapped('container'), $perms[ $container::$modPerm ] ) ) { $canView = FALSE; } } } The tag for a moderator with no forum restrictions is -1, not '*': if ( isset( $perms[ $container::$modPerm ] ) and $perms[ $container::$modPerm ] != -1 )

Leads to It had originally been failing to load the whole page, but I think that's because I had designer mode enabled.

When you hide a topic, the notifications relating to that topic are deleted. However, if there are notifications still queued to be sent (eg because there were a large number of followers), those notifications still end up getting sent, so users receive a notification about a post that they can't see, and the notification doesn't ever get deleted.

Sometimes when a topic is becoming fairly heated, members want to step back and stop replying. However, when they get quoted, they frequently can't resist coming back to continue the argument. It would be really helpful for those users if they could choose to stop receiving quote (and perhaps mention, although that's less prevalent) notifications for a specific post or topic, to help with their self-control.

When databaseCheck is run as part of the 4.7.2 -> 4.7.3 upgrade, it ends up running additional queries to fix the schema, because they weren't included as queries by the upgrader: ALTER TABLE `calendar_events` ADD COLUMN `event_external_url` VARCHAR (255) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci NULL , ADD COLUMN `event_latitude` DECIMAL (10,8) NULL , ADD COLUMN `event_longitude` DECIMAL (11,8) NULL ; This doesn't cause issues because it was caught by the databaseCheck, but as with the previous time that something like this happened: this could cause problems in future.

Error: Call to a member function format() on null (0) #0 /var/www/html/ips-themes/system/Patterns/ActiveRecord.php(335): IPS\calendar\_Event->get__happening() #1 /var/www/html/ips-themes/system/Theme/Theme.php(4620) : eval()'d code(36): IPS\Patterns\_ActiveRecord->__get() #2 /var/www/html/ips-themes/system/Theme/Dev/Template.php(171): IPS\Theme\theme_calendar_front_view_coverPhotoOverlay() #3 /var/www/html/ips-themes/applications/calendar/sources/Event/Event.php(2309): IPS\Theme\Dev\_Template->__call() #4 /var/www/html/ips-themes/applications/calendar/modules/front/calendar/event.php(166): IPS\calendar\_Event->coverPhoto() #5 /var/www/html/ips-themes/system/Dispatcher/Controller.php(118): IPS\calendar\modules\front\calendar\_event->manage() #6 /var/www/html/ips-themes/system/Content/Controller.php(50): IPS\Dispatcher\_Controller->execute() #7 /var/www/html/ips-themes/applications/calendar/modules/front/calendar/event.php(64): IPS\Content\_Controller->execute() #8 /var/www/html/ips-themes/system/Dispatcher/Dispatcher.php(153): IPS\calendar\modules\front\calendar\_event->execute() #9 /var/www/html/ips-themes/index.php(13): IPS\_Dispatcher->run() #10 {main} To repro: Create an event that repeats every 6 years, with a start date in 2019 Try to view that event This is because Event::nextOccurrence only searches within a couple of years, and returns null if it goes out of range, but get__happening doesn't handle null.

Given the prominence of the banner, I would argue it's visual clutter for all but new members, but that does make sense. I'm just not a fan of features only showing up to non-staff, because then we don't form opinions on them (for the same reason, our admins are not exempt from ads). I like the idea of having users choose a topic type, though I'm not sure what the solution would need to look like to be customizable enough (eg different sections have different options).

Search query was "solved"

If a user is a moderator, they never see the solved_did_it_tho_title banner to prompt them to mark a post as the solution, and also don't see the prominent "mark as solution" button. This means we don't know it exists until a member complains about it. This is because you condition both on $topic->isNotModeratorButCanSolve(), when it should really be something like $topic->isNotModeratorButCanSolve() || $topic->starter == \IPS\Member::loggedIn(). As feedback, I would like to be able to configure which forums the banner shows up in, and users get corresponding re-engagement emails - I have solutions turned on for all forums, because it is sometimes useful, but there are only a few where I truly want to push it. // repost from bug report since apparently this is WAI

If a user is a moderator, they never see the solved_did_it_tho_title banner to prompt them to mark a post as the solution, and also don't see the prominent "mark as solution" button. This means we don't know it exists until a member complains about it. This is because you condition both on $topic->isNotModeratorButCanSolve(), when it should really be something like $topic->isNotModeratorButCanSolve() || $topic->starter == \IPS\Member::loggedIn(). As feedback, I would like to be able to configure which forums the banner shows up in, and users get corresponding re-engagement emails - I have solutions turned on for all forums, because it is sometimes useful, but there are only a few where I truly want to push it.

Whoops\Exception\ErrorException thrown with message "Undefined array key "icons_homescreen_maskable"" Stacktrace: #6 Whoops\Exception\ErrorException in /var/www/html/ips/applications/core/modules/admin/customization/icons.php:218 #5 Whoops\Run:handleError in /var/www/html/ips/applications/core/modules/admin/customization/icons.php:218 #4 IPS\core\modules\admin\customization\_icons:processApplicationIcon in /var/www/html/ips/applications/core/modules/admin/overview/onboard.php:183 #3 IPS\core\modules\admin\overview\_onboard:manage in /var/www/html/ips/system/Dispatcher/Controller.php:118 #2 IPS\Dispatcher\_Controller:execute in /var/www/html/ips/applications/core/modules/admin/overview/onboard.php:40 #1 IPS\core\modules\admin\overview\_onboard:execute in /var/www/html/ips/system/Dispatcher/Dispatcher.php:153 #0 IPS\_Dispatcher:run in /var/www/html/ips/admin/index.php:13 URL: /admin/?app=core&module=overview&controller=onboard&initial=1 (on submitting the onboarding wizard) The only action I took during the wizard was to change the site URL (no uploads). I'm IN_DEV.

The nullability analysis doesn't take into account nulls that are explicitly in the global namespace (which is unnecessary but legal). Hook: public function canSetBestAnswer( \IPS\Member $member = \NULL ) Original method: public function canSetBestAnswer( \IPS\Member $member = NULL ) Analysis: [ "method" => "canSetBestAnswer", "reason" => "method_issue_parameters", "parameter" => null, "subclassFile" => "/applications/lmgsys/hooks/bestAnswer_ForumsTopic.php", "baseFile" => "/applications/forums/sources/Topic/Topic.php", "baseClass" => "\IPS\forums\Topic", "subclassMethod" => [ "final" => false, "security" => "public", "static" => false, "name" => "canSetBestAnswer", "parameters" => [ "member" => [ "name" => "member", "type" => "\IPS\Member", "nullable" => false, "passedByReference" => false, "packed" => false, ], ], "returnType" => null, "lineNumber" => 17, ], "subclassName" => "\IPS\forums\lmgsys_hook_bestAnswer_ForumsTopic", "baseMethod" => [ "final" => false, "security" => "public", "static" => false, "name" => "canSetBestAnswer", "parameters" => [ "member" => [ "name" => "member", "type" => "\IPS\Member", "nullable" => true, "passedByReference" => false, "packed" => false, ], ], "returnType" => null, "lineNumber" => 1388, ], "class" => "\IPS\forums\Topic", "priority" => true, ], [ "method" => "canSetBestAnswer", "reason" => "method_issue_nullable", "parameter" => "member", "subclassFile" => "/applications/lmgsys/hooks/bestAnswer_ForumsTopic.php", "baseFile" => "/applications/forums/sources/Topic/Topic.php", "baseClass" => "\IPS\forums\Topic", "subclassMethod" => [ "final" => false, "security" => "public", "static" => false, "name" => "canSetBestAnswer", "parameters" => [ "member" => [ "name" => "member", "type" => "\IPS\Member", "nullable" => false, "passedByReference" => false, "packed" => false, ], ], "returnType" => null, "lineNumber" => 17, ], "subclassName" => "\IPS\forums\lmgsys_hook_bestAnswer_ForumsTopic", "baseMethod" => [ "final" => false, "security" => "public", "static" => false, "name" => "canSetBestAnswer", "parameters" => [ "member" => [ "name" => "member", "type" => "\IPS\Member", "nullable" => true, "passedByReference" => false, "packed" => false, ], ], "returnType" => null, "lineNumber" => 1388, ], "class" => "\IPS\forums\Topic", "priority" => true, ], ],

I'm not proposing that they proactively monitor all exceptions reported by everyone (although I do think that with the right architecture (clearly they should not look at every single individual log) it would be feasible with minimal overhead, and fairly valuable). All I am asking for here is that when someone reports a bug relating to an uncaught exception, IPS support use the logs to see whether it's a widespread issue or just isolated to the reporter, and handle it accordingly. If the logs aren't being used for proactive or reactive error investigation, I'm struggling to see why they bother to collect them at all.

It does also report exceptions 🙂 /** * Should a given exception be reported to IPS? Filter out 3rd party etc. * * @param \Throwable $exception The exception * @return void */ final public static function reportExceptionToIPS( $exception ) { $response = \IPS\Http\Url::external('https://invisionpowerdiagnostics.com')->request()->post( array( 'version' => \IPS\Application::getAvailableVersion('core'), 'class' => \get_class( $exception ), 'message' => $exception->getMessage(), 'code' => $exception->getCode(), 'file' => str_replace( \IPS\ROOT_PATH, '', $exception->getFile() ), 'line' => $exception->getLine(), 'backtrace' => str_replace( \IPS\ROOT_PATH, '', $exception->getTraceAsString() ) ) ); if ( $response->httpResponseCode == 410 ) { \IPS\Settings::i()->changeValues( array( 'diagnostics_reporting' => 0 ) ); } }

All uncaught exceptions are reported to IPS automatically (when opted in), right? Is it really too much to ask that when I report an exception, you check whether that exception is widespread? The problem here is not the changes that I made to this code (which were put in place to address other similar issues, although I don't know if they are still relevant). As far as I can tell, Google made a breaking change to their API to cause this. I'll add it to the list of modifications that I make to IPS files.

TypeError: Cannot access offset of type string on string (0) #0 /opt/forum/system/Login/Handler/OAuth2/Google.php(299): IPS\Login\Handler\OAuth2\_Google->_userData() #1 /opt/forum/applications/core/modules/front/system/settings.php(165): IPS\Login\Handler\OAuth2\_Google->userProfileName() #2 /opt/forum/applications/core/modules/front/system/settings.php(64): IPS\core\modules\front\system\_settings->_overview() #3 /opt/forum/system/Dispatcher/Controller.php(118): IPS\core\modules\front\system\_settings->manage() #4 /opt/forum/applications/core/modules/front/system/settings.php(49): IPS\Dispatcher\_Controller->execute() #5 /opt/forum/system/Dispatcher/Dispatcher.php(153): IPS\core\modules\front\system\_settings->execute() #6 /opt/forum/index.php(13): IPS\_Dispatcher->run() #7 {main} I've not looked into why this is happening, but it seems to be happening fairly frequently. I'm on 4.7.0, but this code doesn't seem to have changed in 4.7.1.