I have to say that the validation function 'valid_int_phone_number($phonenumber)' in phone.int.inc has many issues and doesn't work as proposed.

- Line 31: First digit is removed accidentally (the + is already removed in line 21)
- Line 50: phone_country_code_convert isn't really implemented. Only works for countrycode 1 and 44 (ca and uk)
--> Validation fails for all other countrys, always.

This validation function has to be rewritten to check if the number is a valid E.123 number.
I have to investigate the format then I will write it. (Maybe a good regular expression will do it)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sanjo’s picture

I think this regular expression will fit (source with additonal description http://blog.stevenlevithan.com/archives/validate-phone-number#r4-3):
^\+(?:[0-9] ?){6,14}[0-9]$

I will write the patch now.

stroobl’s picture

FileSize
1.06 KB

I'm also looking into this. I fixed the most important issues in phone.int.inc. Aside from the comments in #1 there is also a typo on line 130. Patch attached. However, I don't like the current approach in phone.inc.int, which depends on phone.countrycode.inc to check the number format for each individual country (line 50: module_load_include('inc', 'phone', 'phone.'. $countrycode);).

The main reason to use the international standard is just to avoid the problems with formatting for individual countries and use a generic solution. If the module maintainer agrees with that, I propose to make a patch to have a basic check for the phone format (like in #1) an remove the dependencies on other country-specific include files.

thierry_gd’s picture

Status: Active » Needs review

Taken into account in 6.2.17 version
Please have a look

stroobl’s picture

It works now, but you missed the
- if ($input_type == 'aplha') {
+ if ($input_type == 'alpha') {
near the end of the file.
However, I don't think that part of the code is used because it works without fixing that - input_type seems to be declared as digits at the start of the function:
function phone_country_code_convert($code, $input_type = 'digits') {

inforeto’s picture

Version: 6.x-2.16 » 6.x-2.17

Checked 6.2.17 and the patch was there.
However, found the following issues:

1. In phone_country_code_convert:

    $codes = array(
      '1' => 'ca',
      '1' => 'us',

For the default of 1 the function returns 'us' instead of 'ca'.
The array now list all country codes but the corresponding files do not exist yet.
This is commented as TODO but breaks the current install of 6.2.17 by silently failing validation.

2. In _normalize_country_code:

    $cc = isset($field['phone_default_country_code']) ? $field['phone_default_country_code'] : '1';

$field['phone_default_country_code'] is set but is still empty if the field is not set to add a default code.
This returns the + sign without a country code that should default to 1 and fail validation.

tedfordgif’s picture

Here's a patch against 2.17 that corrects one problem in phone_country_code_convert, which will now return an array (calling code updated as well). Numeric country codes with multiple two-letter country abbreviations are supported (such as +61 and +1).

Performance Note: I restored the static caching of the $codes array, but call array_keys for the default invocation. If performance is a concern, we could always add a second static cache for the inverse mapping.

tedfordgif’s picture

This patch corrects the validation of international numbers when no country-specific file/function is found. Instead of returning false, it returns true.

--- phone.int.inc       2010-08-19 12:36:44.000000000 -0400
+++ /tmp/phone.int.inc  2010-09-16 10:41:54.000000000 -0400
@@ -44,13 +44,17 @@
   // TODO: Check if parentheses/brackets add up.
   $countrycodes = phone_country_code_convert($cc);
   if (!empty($countrycodes)) {
+    $have_function = FALSE;
     foreach ($countrycodes as $countrycode) {
       $valid_phone_function = 'valid_'. $countrycode . '_phone_number';
       module_load_include('inc', 'phone', 'phone.'. $countrycode);
-      if (function_exists($valid_phone_function) && $valid_phone_function($phonenumber, $field)) {
-        return TRUE;
+      if (function_exists($valid_phone_function)) {
+        $have_function = TRUE;
+        if ($valid_phone_function($phonenumber, $field))
+          return TRUE;
       }
     }
+    return !$have_function;
   }
   return FALSE;
 }
stone_d’s picture

This isnt working. Even i patched everything as its written in this thread, im still getting this error-message

"+49 691705211" is not a valid international phone number
International phone numbers should contain a country code prefixed by a plus sign followed by the local number.

HairMachine’s picture

Same here - mine's a UK number (+44) but I'm getting the error in #8 repeatedly no matter what formatting I attempt.

Ymox’s picture

Same with Swiss phone numbers – every time I need to delete the plus sign and parenthesis, because swiss international phone numbers have 3 more character than what's accepted. (#938248: Rightly formatted values are not accepted)

descender’s picture

I briefly looked through the validation code and noticed that for phone entries without the country code prefix, the validation process will always add +1 regardless of the default country code you set.

The problem is that valid_int_phone_number() always calls _normalize_country_code() without passing the 2nd 'field' array argument:

  $phonenumber = _normalize_country_code($phonenumber);

_normalize_country_code() then never receives any information about the default country code setting (since 'field' is empty) and eventually adds +1 in front:

   $cc = isset($field['phone_default_country_code']) ? $field['phone_default_country_code'] : '1';
   return "+$cc $phonenumber";

The end result -- you're really just validating US phone numbers.

descender’s picture

Just to supplement my last comment, for entries with the country code prefix, there must be a space after the prefix. Otherwise, entries such as '+6561234567' will cause validate_int_phone_number() to fail to find and load the right country-specific validator.

The error is in:

preg_match('/^\+(\d+)/', $phonenumber, $matches);
$cc = $matches[1];
if (strlen($cc) > 3) {
  ...
  return FALSE;
}

$cc will always be > 3.

There is one final problem. Several country-specific validators do not correctly interpret numbers with the country code prefix included. (Note that valid_int_phone_number() will always pass such numbers.)

For example, in valid_sg_phone_number() in phone.sg.inc, the +65 prefix is not recognized since the regex used for validation is:

$regex = "/^[6]\d{7}$/i";

valid_cr_phone_number() in phone.cr.inc seems to have the same problem too. I haven't looked at others.

Danny_Joris’s picture

Same issue here. It doesn't accept Belgian international phone codes. And it doesn't automatically add the country code if not given. (and not giving the country code is the only way I can get the phone number validated)

My client wants to output the phone numbers like this: +32 (0) 12345678 . I'm not sure the brackets are even possible using this module.

All of these example don't validate:
+32 (0) 12345678
+32 012345678
+32 12345678
+3212345678
32012345678
3212345678

The validation error shows this unhelpful message:

"3212345678" is not a valid Belgian phone number
Belgian phone numbers should ...

Edit: I tried the north american phone field as well. The country code does get added there after saving. Unfortunately, when editing and saving the node again, the country code gets cut off again.

inforeto’s picture

One thing though, the international format is the one from the E.123 number, thus that file probably should remain as it is.
(thus not allowing swiss or belgium numbers)

We still need a generic format for all the cases in this thread, maybe as a default on the list of codes.
It also should include the fixes for handling the + signs and for local numbers with no country code.

An alternative would be automatic detection but that'd be another separate issue.

g1smd’s picture

> "My client wants to output the phone numbers like this: +32 (0) 12345678"

There are many reasons why that is a bad idea.
Do not include (0) in the international format.

The only valid display formats are:
+3212345678 - E.164 - should be used for storage.
+32 12 34 56 78 - International format for display.
012 34 56 78 or (012) 34 56 78 - National format for display.

These articles have more detail:
http://www.aa-asterisk.org.uk/index.php/%280%29
http://revk.www.me.uk/2009/09/it-is-not-44-0207-123-4567.html

lukus’s picture

While it might be true that 'valid' display formats have been defined by a standards body - this isn't a priority concern for most users (clients). My feeling is that flexibility is key.

We can't dictate how people enter phone numbers, doing so only creates an unnecessary hurdle for the user.

I'd propose that any parentheses are stripped automatically, and that various field display formats are available to reinsert the parentheses if required.

g1smd’s picture

There are no standards for entry, only for display.

Let users enter whatever they want in whatever format they want,
020 8000 9000
(020) 8000 8000
0208 000 9000
02080 009 000
02080009000
etc. But then, display it in a consistent format, using the right display rules for that country.

A user could enter a US number like 21255-57777, but you'd be daft not to reformat it to 212-555-7777 or +1-212-555-7777 or similar for display.