On Thu, 29 Dec 2016 at 04:10:00, Mark Weiman wrote:
[...] @@ -64,7 +66,7 @@ if (in_request("Action") == "NewAccount") { } } else { print '<p>' . __("Use this form to create an account.") . '</p>'; - display_account_form("NewAccount", "", "", "", "", "", "", "", "", $LANG); + display_account_form("NewAccount", "", "", "", "", "", "", "", "", $LANG, config_get("options", "default_timezone"));
I know I explicitly asked you to add this when reviewing a previous version of this patch. Looking at it again, it seems better to leave this line untouched and handle the default case in the function itself, though.
[...] diff --git a/web/lib/timezone.inc.php b/web/lib/timezone.inc.php new file mode 100644 index 0000000..363d6a6 --- /dev/null +++ b/web/lib/timezone.inc.php @@ -0,0 +1,61 @@ + Displayed Description + */ +function generate_timezone_list() { + $php_timezones = DateTimeZone::listIdentifiers(DateTimeZone::ALL); + + $offsets = array(); + foreach ($php_timezones as $timezone) { + $tz = new DateTimeZone($timezone); + $offset = $tz->getOffset(new DateTime()); + $offsets[$timezone] = "(UTC" . ($offset < 0 ? "-" : "+") . gmdate("H:i", abs($offset)) . + ") " . $timezone; + } + + asort($offsets); + return $offsets; +} + +/** + * Set the $TZ global variable. + * + * @global string $TZ Timezone set for session. + * + * @return null + */ +function set_tz() { + global $TZ; + + $timezones = generate_timezone_list(); + + if (isset($_COOKIE['AURSID'])) { + $dbh = DB::connect(); + $q = "SELECT Timezone FROM Users, Sessions "; + $q .= "WHERE Users.ID = Sessions.UsersID "; + $q .= "AND Sessions.SessionID = "; + $q .= $dbh->quote($_COOKIE['AURSID']); + $result = $dbh->query($q);
I wonder whether it is better to store the timezone in a cookie. The set_tz() function could query the database the first time a user logs in only, set the cookie and become a no-op for any subsequent calls. By doing that, we could also get rid of the global variable and the patch could be extended easily such that unregistered users can specify a timezone.
+ + if ($result) { + $row = $result->fetchAll(); + $TZ = $row[0][0]; + } else { + $TZ = config_get("options", "default_timezone"); + } + + if (!array_key_exists($TZ, $timezones)) { + $TZ = config_get("options", "default_timezone"); + } + } + + if (isset($TZ) && $TZ != "") { + date_default_timezone_set($TZ); + } else { + date_default_timezone_set(config_get("options", "default_timezone")); + } +}
This part can be simplified as follows (untested and assuming that we replace the global variable by a local variable as suggested above): unset($timezone); if ($result) { $timezone = $result->fetchColumn(0); } if (!isset($timezone) || !array_key_exists($timezone, $timezones)) { $timezone = config_get("options", "default_timezone"); } date_default_timezone_set($timezone); The rest looks fine to me, thanks!