Timezone Detect is a lightweight Drupal 7 module that leverages the jsTimezoneDetect library for automatic detection and setting of a user's timezone via javascript. It can set a user's timezone automatically upon first login, and update it on every login if desired.

Sandbox: http://drupal.org/sandbox/JordanMagnuson/1933830
Git: JordanMagnuson@git.drupal.org:sandbox/JordanMagnuson/1933830.git

IMPORTANT: for this module to work, you must download jstz.js and place it in sites/all/modules/timezone_detect/jstz.js (or alternatively at sites/all/libraries/jstimezonedetect/jstz.js if you have Libraries API 2.x installed).

Benefits of automatic timezone detection

The setting of user timezones is often fraught with confusion and frustration.

To start with, some users will never update their timezone settings manually, even when prompted to do so at every login; I know this from experience running a large website where timezone settings are an important factor. These same users will sometimes complain about confusions caused by incorrect timezone settings, though they have not bothered to update their accounts.

The users who DO follow through on updating their timezone settings are often confused about which timezone they inhabit, and which timezone they should select--even when provided with a map to click on. The Olson timezone codes (e.g. "America/Chicago") are not immediately obvious to everyone, and some users are confused when they do not see their particular city listed as an option.

This module mitigates these kinds of issues by setting a sane "best guess" default timezone for every user at first login, so that you can:

  1. Be confident that dates and times are always displayed correctly for all users.
  2. Carry out time-sensitive cron tasks at the best time for all users (e.g. updating credits overnight, sending out emails in the morning).
  3. Avoid the common confusions that arise when people are asked to set their timezones manually.

Similar Modules

The module is similar in intent to Auto Time Zone, but differs in that it is leveraging an outside javascript library, rather than rolling its own javascript solution. jsTimezoneDetect is an excellent library that is well maintained (see issue queue), and my module will allow users to get updates to the library independent of module updates.

Other considerations:

  1. Auto Time Zone only has a stable release for Drupal 5.x, and has not been maintained or updated for a long time.
  2. I think Timezone Detect is a more precise name than Auto Time Zone for my particular module's goals, and it's relationship to the jsTimezoneDetect library.

Comments

Asmon’s picture

Asmon commented

Hello Jordan,

  1. Please try to fix issues raised by automated review tools: http://ventral.org/pareview/httpgitdrupalorgsandboxjordanmagnuson1933830git
  2. Your are currently working in master branch. Please move branch to 7.x-1.x and delete the master branch. see http://drupal.org/node/1127732
  3. You need to remove your LICENSE.txt because Drupal will add it automaticaly on packaging. see http://drupal.org/node/1587704

My feedback:

  • You can use curl to check if javascript file is reachable.
  • You can improve the admin configuration by adding every constant in a form in case the path change.

Regards.

mikespence’s picture

mikespence commented
Status: Needs review » Needs work

Hello,

Great idea for a module. This would certainly have been useful for me in a recent project, hoping this comes all the way through.

  • I would check in here: http://ventral.org/pareview/httpgitdrupalorgsandboxjordanmagnuson1933830git for a code review on how to meet the Drupal standards. However, don't be put off by how much is there, it's all relatively simple things.
  • You're working on a master branch (I did the same with my module when I started it!), it's easy to remove. Just look here: http://drupal.org/node/1127732
  • The LICENSE file is added by Drupal so that can be removed.
  • README file - brilliant amount of detail but make sure your lines don't exceed 80 characters.
  • Your inline comments have to end with a full stop (for example line 7 of your js file)

I hope these help you get your module underway, like I said, I'd quite like to use this when it's finished!

Mike

jordanmagnuson’s picture

jordanmagnuson commented
Status: Needs work » Needs review

Well, that was embarrassing. And educational. I ran my code through Coder, but forgot to install CodeSniffer... so.

The code is now passing all the automated tests. Thank you both for your input.

@Asmon: I will look into your suggestions!

jordanmagnuson’s picture

jordanmagnuson commented
Issue summary: View changes

Add information about downloading library

samail’s picture

samail commented
Status: Needs review » Needs work

Hi JordanMagnuson,

* Automated test succesfully passed.

* After installing the module, I've got a Notice message "Notice: Undefined variable: admin in timezone_detect_admin_form() (line 21 of .../sites/all/modules/timezone_detect/timezone_detect.admin.inc)."

* Nothing wrong in Watchdog except above notice so that is almost good.

* After a quick review, code looks pretty good and well documented ;-)
I agree with Asmon's suggestion about improving the admin console to be able to edit files path.

jordanmagnuson’s picture

jordanmagnuson commented
Status: Needs work » Needs review

Thanks for the feedback samail!

  1. I've fixed the undefined variable notice/bug.
  2. I've enabled editing the library path information via an admin interface at admin/config/regional/timezone_detect/library_info.
  3. I've decided not to implement cURL, since the whole point of checking the remote file is reading it for version information. fopen() is a bit slower, but works fine, and I cache the results for a week, so I prefer not to require the cURL extension unnecessarily. (Note that if the remote file doesn't exist for some reason, or is unreadable, fopen() will return FALSE, and my module handles the situation without a problem.)
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and put yourself on the PAReview: review bonus high priority list. Then I'll take a look at your project right away :-)

jordanmagnuson’s picture

jordanmagnuson commented

To those who have looked at the project so far, please let me know if there are any further outstanding issues. Otherwise I would be obliged if someone could mark this RTBC.

The codebase is very small, and I've been using this module on a production site with 10,000 users for a couple of weeks now.

Thanks!

jordanmagnuson’s picture

jordanmagnuson commented

[Accidental double post.]

mitchell’s picture

mitchell commented
Status: Needs review » Needs work

I tried testing on simplytest.me, but I could not get it to work. After changing the settings to update on each login, I logged out and logged back in, but the timezone didn't update.

On the 'Library Information' page, I received this error:

Notice: Undefined variable: options in timezone_detect_library_info_form() (line 38 of /home/s987488368603815/www/sites/default/modules/1933830/timezone_detect.admin.inc).
Notice: Undefined variable: options in timezone_detect_library_info_form() (line 45 of /home/s987488368603815/www/sites/default/modules/1933830/timezone_detect.admin.inc).
Notice: Undefined variable: options in timezone_detect_library_info_form() (line 52 of /home/s987488368603815/www/sites/default/modules/1933830/timezone_detect.admin.inc).
jordanmagnuson’s picture

jordanmagnuson commented
Status: Needs work » Needs review

Thanks for catching that error message, Mitchell: that crept in on my last update.

Regarding the module not working, I don't think you'll be able to test it properly on simplytest.me. As stated in my description above, in the readme, and on the 'site status' page:

IMPORTANT: for this module to work, you must download jstz.js and place it in sites/all/modules/timezone_detect/jstz.js (or alternatively at sites/all/libraries/jstimezonedetect/jstz.js if you have Libraries API 2.x installed).

mitchell’s picture

mitchell commented

JordanMagnuson, #1031450: Use Geolocation API for timezone detection may also be of interest to you.

mitchell’s picture

mitchell commented
Assigned: Unassigned » patrickd

I tried to make a more thorough pass, so here's a bit more feedback:

Before enabling libraries, I received this error (due to the webenabled host):

Warning: fopen() [function.fopen]: URL file-access is disabled in the server configuration in _timezone_detect_get_version() (line 124 of .../sites/all/modules/contrib/timezone_detect/timezone_detect.install).
Warning: fopen(https://bitbucket.org/pellepim/jstimezonedetect/raw/default/jstz.js) [function.fopen]: failed to open stream: no suitable wrapper could be found in _timezone_detect_get_version() (line 124 of .../sites/all/modules/contrib/timezone_detect/timezone_detect.install).

I was trying to install the library manually, so these remote calls were unexpected. I think instead of performing these, you could give users more detailed info through the install process. Also, I had the js file in the timezone_detect module folder, so I had expected it to work without libraries enabled.

I found the 'Library Information' settings tab to be mostly confusing. Shouldn't these variables be hardcoded anyway?

I'm not familiar with common practices for modules integrating a js lib, so much of the install file was unfamiliar, but I think patrickd might have some ideas.

@patrickd: What do you think of this module's methods for ensuring the dependent library is available?

Overall, it works and is well documented. Thanks, JordanMagnuson.

jordanmagnuson’s picture

jordanmagnuson commented

@mitchell: currently the jsTimezoneDetect library must be installed locally in order for this module to work. It will work with or without the Libraries API module installed (the remote calls have nothing to do with Libraries API). The remote calls are to gain up-to-date version information about the library. This allows site administrators to be informed about library updates via the status report page, and install updates should they choose to do so. Did you take a look at the status report page after installing?

I think the warning message you received is reasonable. If fopen() fails to open the remote url based on server restrictions, it should return the reason for the failure. This will not affect the end user experience as long as "error messages to display" are set to "none" at admin/config/development/logging (as recommended for a production site). The indication of the failure will then simply be on the status report page, which will display "no update data available" as intended.

Note that the module only checks for remote version information once a week and caches the result, so there is no danger of fopen() error messages flooding watchdog if working with in a restricted environment.

Regarding the "library information" tab in the admin settings, I agree that it is confusing, and think it should be removed. I added that tab in response to reviewer suggestion (see comments above), but I don't think it was a good idea. That information should indeed always be hard-coded, and if those remote library variables were to change, the module should be updated...

jordanmagnuson’s picture

jordanmagnuson commented

FYI, the methods used for detecting and informing on remote library version information in this module are essentially identical to those used in the Timeago module.

mitchell’s picture

mitchell commented
Assigned: patrickd » Unassigned
Status: Needs review » Fixed

Sounds good, Jordan. You'll get more feedback in full project status, so I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks to the dedicated reviewer(s) as well.

jordanmagnuson’s picture

jordanmagnuson commented

Thanks Mitchell!

jordanmagnuson’s picture

jordanmagnuson commented

I got hung up on another project for a little bit, but I finally got around to releasing a 1.0 version of Timezone Detect: http://drupal.org/project/timezone_detect

Removed the Libraries API dependency, so the module now works right out of the gate after downloading and enabling it.

Cheers to all of you for the feedback/assistance.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Changed git link.