[Pushed] Delayed moderation - time bug

[Eldenroot]

IT should take this setting. If not then it is waste of time to set anything here

[Evenprime]

I've looked through the code a bit. The delayed moderation seems to currently be the only place where input of time is needed/possible and explicitely expecting "am/pm". The Calendar and other locations only have date inputs, therefore this time problem doesn't affect them. A fix for this problem therefore should only affect moderation.php.

I'd volunteer to fix this problem if it gets pushed to Github and the intended solution is chosen.

Respecting the timezone and optional DST of the moderating user account settings should be a mandatory part of the fix (imho), as everything else is just confusing.

Beyond that I have two suggestions of different size:

Big impact: Using dropdown menus for all inputs: the day and month are already dropdowns, The year could be one (offering current and next 4 years just like the calendar already does). Hours would be a dropdown with numbers 00-23, Minutes a dropdown with numbers 00-59. From context everybody should understand what they are expected to enter. Optionally the "hours" dropdown could use labels with both notations like "23 (11 pm)". Or it depends on the time display setting of the admin cp, if the am/pm or the 24 notation is used.
One problem with this approach is, that it requires template modifications and the delayed moderation feature will be broken for everybody who doesn't update the templates, for no obivous reason.

Small impact: The input fields all stay the same, but the time input field uses the time format setting of the admin cp. And it expects the input to be sent back in the exact same format. As long as people have valid time format settings in their admin-cp, it should be no problem to parse the inputs to extract the hours and minutes again afterwards.

[Destroy666]

The delayed moderation seems to currently be the only place where input of time is needed/possible and explicitely expecting "am/pm". The Calendar and other locations only have date inputs, therefore this time problem doesn't affect them. A fix for this problem therefore should only affect moderation.php.

Don't know where and how you looked, but that's just incorrect information.
Announcements: https://docs.google.com/file/d/0B6pgReiH...p=drivesdk
ACP announcements: https://docs.google.com/file/d/0B6pgReiH...p=drivesdk
Edit calendar event: https://docs.google.com/file/d/0B6pgReiH...p=drivesdk
And maybe more.
EDIT:
Also found it in calendar weekview: https://docs.google.com/file/d/0B6pgReiH...p=drivesdk

Feel free to submit a PR for all of them though.

[Evenprime]

You are right. I can't believe I've not seen those.

I'll definitely take a close look at all of these, but so far these seem to at least work as declared:

The calendar does accept both am/pm and 23:59 formatted inputs and correctly interprets them respecting the timezone set in the dropdown menu. Not sure about DST handling though. As timezones are explicitely handled at this location, I'd just leave it as is. The time is also correctly displayed in all locations based on the admin cp "timeformat" setting, even the weekly view.

The admin-cp announcements already have a hint about GMT, therefore they probably should stay unchanged. It also does accept both time formats.

The mod-cp announcements accept both time formats and interprets them as GMT (just like the ACP version), but there is no hint about that. I think that the functionality should be kept consistent with the ACP version and therefore only a hint should be added about GMT to the mod-cp version.

Delayed moderation also accepts 23:59 formatted time as input in addition to the 11:59 pm format. So I'll leave the input parsing as is and just add timezone handling at this location, as that is probably the expected behaviour of this feature, despite using GMT everywhere else.

The initial time formats for the input fields vary throughout the forum. The calendar already respects the "timeformat" setting of the Admin-CP, delayed moderation and announcements don't. That should be easy to change to use the "timeformat" setting everywhere.

I'll open a pull request when I'm done.

[Eldenroot]

Thank you!

[Evenprime]

https://github.com/mybb/mybb/pull/1795

I've created the pull request. Copy-Pasta of pull request description:

The target of this pull request was two-fold:

   Respect the "$mybb->settings['timeformat']" value (set in Admin-CP) at all locations that display time or allow the input of time as text (and don't/can't use my_date for date/time formatting). This affects:
   Admin-CP Forum Announcement creation/editing
   Admin-CP User banning preset bantimes selectbox entries
   Admin-CP Mass Mail creation/editing
   Mod-CP Forum Announcement creation/editing
   Delayed Moderation

   Respect current user timezone and dst when entering date/time. This affects:
   Admin-CP Mass Mail creation/editing
   Mod-CP Forum Announcement creation/editing
   Delayed Moderation (original reason for this pull request as mentioned in the corresponding thread)
   (Admin-CP Forum Announcement creation was intentionally left "GMT" only, because there is a label that states that inputs are always interpreted as GMT.)
   (The Calendar already offers to set the used timezone explicitely, therefore I didn't touch the logic there. I only shortened the time patterns at two locations, because the hour/minute part that is generated isn't used at these locations at all.)

   In addition minor related bugs are fixed:
   Admin-CP Mass Mail added the current users timezone when saving data. If at all, the timezone data should have been subtracted at that point. This bug lead to strange behaviour, where repeatedly editing and saving the same mass-mail would push its delivery time further into the future (when timezone offset is positive) or the past (the other case).
   Admin-CP Mass Mail would interpret "11:00 PM" as "11:00", because the matching for "pm" only checked for lowercase "pm". Everywhere else the same logic was case insensitive, now it is here too.
   The function get_event_date($event) in functions.php would have applied timezones twice, if the server has a timezone/location set that's not "+0", because mktime is used where gmmktime should've been used (The correct timezone information is applied one line later during a call to "my_date"). This method seems to not be used at all currently.

Looking forward to comments/suggestions for changes/improvements.

[Destroy666]

Hi,

Thank you for your report. We have pushed this issue to our Github repository for further analysis where you can track our commits and progress with fixing this bug. Discussions regarding this bug may also take place there too.

Follow this link to visit the issue on Github: https://github.com/mybb/mybb/issues/1796

Thanks for contributing to MyBB!

Regards,
The MyBB Group