Improved parsing of international numbers

Added error message to description.

Patch did not apply to current 7.x-1.x, rerolled the patch with a small change:
< - drupal_set_message('langue cc = ' . $cc, 'error');
> - //drupal_set_message('langue cc = ' . $cc, 'error');

Note that I do not want to take credit for this contribution.

I just ran into the same problem. Thanks a lot for the patch, it seems to work fine for me now! Without this patch, this module seems hardly usable (with international numbers), since it is very unobvious for a user that the space is needed and the error message leads into a totally wrong direction. It'd be great if the patch could be integrated.

Patch is working absolute fine for me as well.
Setting this to RTBC so that it'll hopefully be committed soon.

Previous patch didn't apply for me, and introduced weird whitespace, so I re-rolled it.

I'm tempted to set this back to Needs Review, because of #2.

Notes and discoveries:

#1
the huge regex isn't necessary. I'm pretty sure a simpler solution using substr will save memory (and possibly CPU).

#2
the array in phone_country_code_convert() has duplicate keys, so that needs fixing. I placed 'us' before 'ca' so 'us' is returned for input value 1, because that is also the behavior from before the patch. 'au' is now returned for 61 and 'it' for 39 (instead of 'cc' and 'va' respectively) because that just seems sensible.

Note that means that phone.ca.inc is never used. But that seems to be the case already!?
That's why I didn't fix this (this can be done by someone else if they want to) - I only provided clear comments.

#3
the patch makes numbers actually be validated. Before the patch, TRUE is almost always returned, because the module_load_include() is wrong, so $valid_phone_function is not called. (Unless it was loaded already.)

#4
When we're starting to validate things anyway... I changed the 'else' case to not only return TRUE, but check (using format_int_phone_number()) whether an international dialing code is present in the area code, and return FALSE if not.
(If this check isn't done, then format_int_phone_number() will still delete figures without warning you, leading to bugs like #1563692: format_int_phone_number() fails to use the appropriate country formatter and strips bits out of valid Swedish phone numbers.)

.

EDIT:

Scratch that, I was testing on the wrong system. The patch seems to work; I no longer get an Italian-specific error message when putting in random garbage. I haven't tested further, but I can confirm that part works fine, now.

However the string is missing a period at the end of the sentence.

I am using 7.x-1.0-beta1 version of Phone module, can I use the patch against this beta version?

Also, which patch to use? Confused between #1, #5 and #7?

Thanks.

+++ b/include/phone.int.inc
@@ -37,33 +37,40 @@ function valid_int_phone_number($phonenumber) {
+  // Check for correct country code.
+  // TODO: Fix problems with shared country codes. For instance, $countrycode is
+  //       never 'ca' (but always 'us'). One of the following things needs done:
+  //       - all countries sharing the same area need to be validated in the
+  //         same .inc file (e.g. phone.1.inc or phone.61.inc)
+  //       - there will need to be a separate 'pre-validate' function that
+  //         determines which country a phone number belongs to.
+  $codes = array_flip(array_unique(phone_country_codes()));
+  foreach (array(1, 2, 3) as $length) {
+    $nr_start = substr($phonenumber, 1, $length);
+    if (isset($codes[$nr_start])) {
+      $countrycode = $codes[$nr_start];
+      break;
+    }
+  }

How about not using Country Codes? US and Canada are using north american standards, should there be one file for them both. NA/North America.

Patch #7 did fix the wrong validation error.

I think this approach is fine, it's better to work from something tangible as country codes that might come up in other scenarios.

I made a small modification to patch 7 so that in the case where we don't have a specific validation function for a given country then we at least check that the number is more than 5 digits. Patch attached.

Fixed patch with the correct variable name

@lambic this was RTBC already, could you provide an interdiff with your change so we can review the change, hopefully worth the status change?

See https://www.drupal.org/documentation/git/interdiff

Thanks for the interdiff. Why 5 digits? Isn't 911 or 411 valid north american phone numbers?

This is in international validation, so only happens if there's no country specific validation. In theory for countries with a single digit country code and no country specific validation, 3 digit phone numbers would fail, we could reduce the check to 4 digits to cover that use case, or make the number of digits settable via a variable.

All the number without country code will call valid_us_phone_number and include/phone.us.inc file doesn't exist.