From 5ba64efd7c426b99b4bcbad7a9b841daf13eba06 Mon Sep 17 00:00:00 2001 From: Paul Dexter-Sobkowiak <536437+krubokrubo@users.noreply.github.com> Date: Mon, 26 Nov 2018 22:15:19 +0000 Subject: [PATCH] Show tel: links for multiple phone numbers separated by ; Closes #2069 --- app/helpers/browse_helper.rb | 40 +++++++++---- test/helpers/browse_helper_test.rb | 91 ++++++++++++++++++++---------- 2 files changed, 91 insertions(+), 40 deletions(-) diff --git a/app/helpers/browse_helper.rb b/app/helpers/browse_helper.rb index eebe1af8f..e6ac242dd 100644 --- a/app/helpers/browse_helper.rb +++ b/app/helpers/browse_helper.rb @@ -74,8 +74,12 @@ module BrowseHelper safe_join(wdt, ";") elsif url = wiki_link("tag", "#{key}=#{value}") link_to h(value), url, :title => t("browse.tag_details.wiki_link.tag", :key => key, :value => value) - elsif url = telephone_link(key, value) - link_to h(value), url, :title => t("browse.tag_details.telephone_link", :phone_number => value) + elsif phones = telephone_links(key, value) + # similarly, telephone_links() returns an array of phone numbers + phones = phones.map do |p| + link_to(h(p[:phone_number]), p[:url], :title => t("browse.tag_details.telephone_link", :phone_number => p[:phone_number])) + end + safe_join(phones, "; ") else linkify h(value) end @@ -178,14 +182,28 @@ module BrowseHelper nil end - def telephone_link(_key, value) - # does it look like a phone number? eg "+1 (234) 567-8901 " ? - return nil unless value =~ %r{^\s*\+[\d\s\(\)/\.-]{6,25}\s*$} - - # remove all whitespace instead of encoding it http://tools.ietf.org/html/rfc3966#section-5.1.1 - # "+1 (234) 567-8901 " -> "+1(234)567-8901" - value_no_whitespace = value.gsub(/\s+/, "") - - "tel:#{value_no_whitespace}" + def telephone_links(_key, value) + # Does it look like a global phone number? eg "+1 (234) 567-8901 " + # or a list of alternate numbers separated by ; + # + # Per RFC 3966, this accepts the visual separators -.() within the number, + # which are displayed and included in the tel: URL, and accepts whitespace, + # which is displayed but not included in the tel: URL. + # (see: http://tools.ietf.org/html/rfc3966#section-5.1.1) + # + # Also accepting / as a visual separator although not given in RFC 3966, + # because it is used as a visual separator in OSM data in some countries. + if value =~ %r{^\s*\+[\d\s\(\)/\.-]{6,25}\s*(;\s*\+[\d\s\(\)/\.-]{6,25}\s*)*$} + return value.split(";").map do |phone_number| + # for display, remove leading and trailing whitespace + phone_number = phone_number.strip + + # for tel: URL, remove all whitespace + # "+1 (234) 567-8901 " -> "tel:+1(234)567-8901" + phone_no_whitespace = phone_number.gsub(/\s+/, "") + { :phone_number => phone_number, :url => "tel:#{phone_no_whitespace}" } + end + end + nil end end diff --git a/test/helpers/browse_helper_test.rb b/test/helpers/browse_helper_test.rb index 8890a0de6..acc7f2727 100644 --- a/test/helpers/browse_helper_test.rb +++ b/test/helpers/browse_helper_test.rb @@ -123,6 +123,9 @@ class BrowseHelperTest < ActionView::TestCase html = format_value("phone", "+1234567890") assert_dom_equal "+1234567890", html + html = format_value("phone", "+1 (234) 567-890 ; +22334455") + assert_dom_equal "+1 (234) 567-890; +22334455", html + html = format_value("wikipedia", "Test") assert_dom_equal "Test", html @@ -304,48 +307,78 @@ class BrowseHelperTest < ActionView::TestCase assert_nil link end - def test_telephone_link - link = telephone_link("foo", "Test") - assert_nil link + def test_telephone_links + links = telephone_links("foo", "Test") + assert_nil links - link = telephone_link("phone", "+123") - assert_nil link + links = telephone_links("phone", "+123") + assert_nil links - link = telephone_link("phone", "123") - assert_nil link + links = telephone_links("phone", "123") + assert_nil links - link = telephone_link("phone", "123 abcdefg") - assert_nil link + links = telephone_links("phone", "123 abcdefg") + assert_nil links - link = telephone_link("phone", "+1234567890 abc") - assert_nil link + links = telephone_links("phone", "+1234567890 abc") + assert_nil links - link = telephone_link("phone", "+1234567890; +22334455667788") - assert_nil link + # If multiple numbers are listed, all must be valid + links = telephone_links("phone", "+1234567890; +223") + assert_nil links - link = telephone_link("phone", "1234567890") - assert_nil link + links = telephone_links("phone", "1234567890") + assert_nil links - link = telephone_link("phone", "+1234567890") - assert_equal "tel:+1234567890", link + links = telephone_links("phone", "+1234567890") + assert_equal 1, links.length + assert_equal "+1234567890", links[0][:phone_number] + assert_equal "tel:+1234567890", links[0][:url] - link = telephone_link("phone", "+1234-567-890") - assert_equal "tel:+1234-567-890", link + links = telephone_links("phone", "+1234-567-890") + assert_equal 1, links.length + assert_equal "+1234-567-890", links[0][:phone_number] + assert_equal "tel:+1234-567-890", links[0][:url] - link = telephone_link("phone", "+1234/567/890") - assert_equal "tel:+1234/567/890", link + links = telephone_links("phone", "+1234/567/890") + assert_equal 1, links.length + assert_equal "+1234/567/890", links[0][:phone_number] + assert_equal "tel:+1234/567/890", links[0][:url] - link = telephone_link("phone", "+1234.567.890") - assert_equal "tel:+1234.567.890", link + links = telephone_links("phone", "+1234.567.890") + assert_equal 1, links.length + assert_equal "+1234.567.890", links[0][:phone_number] + assert_equal "tel:+1234.567.890", links[0][:url] - link = telephone_link("phone", " +1234 567-890 ") - assert_equal "tel:+1234567-890", link + links = telephone_links("phone", " +1234 567-890 ") + assert_equal 1, links.length + assert_equal "+1234 567-890", links[0][:phone_number] + assert_equal "tel:+1234567-890", links[0][:url] - link = telephone_link("phone", "+1 234-567-890") - assert_equal "tel:+1234-567-890", link + links = telephone_links("phone", "+1 234-567-890") + assert_equal 1, links.length + assert_equal "+1 234-567-890", links[0][:phone_number] + assert_equal "tel:+1234-567-890", links[0][:url] - link = telephone_link("phone", "+1 (234) 567-890") - assert_equal "tel:+1(234)567-890", link + links = telephone_links("phone", "+1 (234) 567-890") + assert_equal 1, links.length + assert_equal "+1 (234) 567-890", links[0][:phone_number] + assert_equal "tel:+1(234)567-890", links[0][:url] + + # Multiple valid phone numbers separated by ; + links = telephone_links("phone", "+1234567890; +22334455667788") + assert_equal 2, links.length + assert_equal "+1234567890", links[0][:phone_number] + assert_equal "tel:+1234567890", links[0][:url] + assert_equal "+22334455667788", links[1][:phone_number] + assert_equal "tel:+22334455667788", links[1][:url] + + links = telephone_links("phone", "+1 (234) 567-890 ; +22(33)4455.66.7788 ") + assert_equal 2, links.length + assert_equal "+1 (234) 567-890", links[0][:phone_number] + assert_equal "tel:+1(234)567-890", links[0][:url] + assert_equal "+22(33)4455.66.7788", links[1][:phone_number] + assert_equal "tel:+22(33)4455.66.7788", links[1][:url] end def add_old_tags_selection(old_node) -- 2.39.5