The PIM Manager can use normalized phone numbers in EDS for two things: - avoid calculating the normalized numbers after reading a contact - search via normalized string when doing a direct search
The problem at the moment is that the normalized phone number support in the "openismus-work" branch is unusable resp. out-dated. 1. The API is not the same as in master. It would be good to not require different code in SyncEvolution for openismus-work and master. Differences are: * e_book_client_connect_direct in master, e_book_client_new_direct in openismus-work. * Phone numbers split into two parts in master 2. Importing SyncEvolution test cases into openismus-work crashes evolution-addressbook-factory in code related to phone numbers. Because libebook's synchronous APIs do not notice that crash, SyncEvolution hangs. To reproduce: SYNCEVOLUTION_DEBUG=1 /home/pohly/work/syncevolution/src/syncevolution --daemon=no --import ~pohly/src/syncevolution/test/testcases/eds_contact.vcf backend=evolution-contacts Note that this depends on the actual data. The initial 11 contacts import okay, then it fails for "John Doe". evolution-addressbook-factory fails with: (evolution-addressbook-factory:27057): libedata-book-ERROR **: e-book-backend-sqlitedb.c:1462 (convert_phone): Not a phone number It must not assume that all TEL values are valid phone numbers! Furthermore, why is this code used? The SyncEvolution command above creates and uses the system book without enabling any extensions. Is the support for normalized phone numbers always enabled if EDS was compiled accordingly, or does it depend on some extension?
In the master branch, EDS no longer aborts when processing an invalid phone number. Instead, a warning is printed: (evolution-addressbook-factory:19402): libedata-book-WARNING **: e-book-backend-sqlitedb.c:1654: 'primary 8': Not a phone number I think even that warning should not be printed by default. It may be useful for debugging, but is too verbose for normal production.
The 3rd problem is that traditional TEL string searches no longer work as before. The expectation is that a suffix search with E_CONTACT_TEL E_BOOK_QUERY_ENDS_WITH "8899" will not match a TEL:+49-89-7888 99. The patch series for libphonenumber seems to change that such that the example above also matches. I'd like to understand why.
And the 4th problem. Found when using EDS master. To reproduce (assuming that you use jhbuild to set up EDS master, as I do): cd /tmp/; mkdir -p testing; cd testing; export XDG_CONFIG_HOME=/tmp/testing/temp-testpim/config XDG_DATA_HOME=/tmp/testing/temp-testpim/local/cache; jhbuild run ~pohly/src/syncevolution/test/dbus-session.sh bash PATH=/home/pohly/work/syncevolution/src:$PATH ~pohly/src/syncevolution/src/dbus/server/pim/testpim.py -v TestContacts.testFilterStartup 2>&1 | tee /tmp/log The testFilterStartup adds three contacts: u'''BEGIN:VCARD VERSION:3.0 N:Zoo;Abraham NICKNAME:Ace TEL:1234 TEL:56/78 TEL:+1-800-FOOBAR TEL:089/788899 EMAIL:az@example.com END:VCARD''', u'''BEGIN:VCARD VERSION:3.0 N:Yeah;Benjamin TEL:+49-89-788899 END:VCARD''', u'''BEGIN:VCARD VERSION:3.0 FN:Charly 'Chárleß' Xing N:Xing;Charly END:VCARD''' It runs a (endswith "phone" "8899") query against EDS. The expection is, as in testFilterStartupRefine, that it gets two contacts back. That fails when using DRA, which is enabled by default: Traceback (most recent call last): File "/home/pohly/src/syncevolution/src/dbus/server/pim/testpim.py", line 3195, in testFilterStartup self.assertEqual(2, len(self.view.contacts)) File "/home/pohly/src/syncevolution/test/testdbus.py", line 1321, in assertEqualCustom unittest.TestCase.assertEqual(self, self.stripDBus(a, sortLists), self.stripDBus(b, sortLists), msg) AssertionError: 2 != 0 That's this line, which means "two contacts expected, zero actual content": self.assertEqual(2, len(self.view.contacts)) When disabling DRA via SYNCEVOLUTION_NO_PIM_EDS_DIRECT=1, everything works as expected: SYNCEVOLUTION_NO_PIM_EDS_DIRECT=1 PATH=/home/pohly/work/syncevolution/src:$PATH ~pohly/src/syncevolution/src/dbus/server/pim/testpim.py -v TestContacts.testFilterStartup 2>&1 | tee /tmp/log testFilterStartup (__main__.TestContacts) TestContacts.testFilterStartup - phone number lookup while folks still loads ... ok You can check testFilterStartup_TestContacts.dbus.log to see the EDS query. It's not logged when using DRA. Note that you can run all tests when passing "TestContacts" to testpim.py or select some of the other failing tests, like TestContacts.testFilterStartupRefine. In that test, the contact data is different. As mentioned before, now the same search finds *too many* matches (two instead of just one). Enabling/disabling DRA has no effect.
(In reply to comment #3) > The 3rd problem is that traditional TEL string searches no longer work as > before. The expectation is that a suffix search with E_CONTACT_TEL > E_BOOK_QUERY_ENDS_WITH "8899" will not match a TEL:+49-89-7888 99. Fixed in master.
(In reply to comment #4) > And the 4th problem. [...] > It runs a (endswith "phone" "8899") query against EDS. The expection is, as > in testFilterStartupRefine, that it gets two contacts back. That fails when > using DRA, Fixed in master. The problem was that client and server had different XDG dirs set, and the code which was meant to handle that didn't work. As a result, the client created its own, empty contact.db and read from there.
5th problem: after switching to E_BOOK_QUERY_EQUALS_PHONE_NUMBER with a E164 caller ID as comparison value (includes country code!), contacts which have no country code don't match. The query, visible in the server output and the D-Bus log when using SYNCEVOLUTION_NO_PIM_EDS_DIRECT=1 is: (eqphone "phone" "+4989788899" "de_DE.UTF-8") This query is expected to match this contact: VERSION:3.0 X-EVOLUTION-FILE-AS:Zoo\, Abraham N:Zoo;Abraham NICKNAME:Ace TEL;X-EVOLUTION-E164=1234,:1234 TEL;X-EVOLUTION-E164=5678,:56/78 TEL;X-EVOLUTION-E164=800366227,"+1":+1-800-FOOBAR TEL;X-EVOLUTION-E164=89788899,:089/788899 EMAIL:az@example.com UID:pas-id-51272A3C000000BD REV:2013-02-22T08:20:12Z(331) FN:Abraham Zoo END:VCARD But it doesn't. It matches only the contact which has +49 explicitly set: BEGIN:VCARD VERSION:3.0 X-EVOLUTION-FILE-AS:Yeah\, Benjamin N:Yeah;Benjamin TEL;X-EVOLUTION-E164=89788899,"+49":+49-89-7888 99 UID:pas-id-51272A3C000000BE REV:2013-02-22T08:20:12Z(333) FN:Benjamin Yeah END:VCARD To reproduce, use for-master/pohly as committed today. The relevant change is this: commit 1330644b2d45d2ab24ee9708d29f1daead316e4f Author: Patrick Ohly <patrick.ohly@intel.com> Date: Fri Feb 22 08:56:13 2013 +0100 PIM: intelligent phone search in EDS (FDO #59571, part 2) If phone number search is possible, then the direct search in EDS now uses the more accurate E_BOOK_QUERY_EQUALS_PHONE_NUMBER comparison, with the E164 formatted caller ID as value to compare against. That value includes the current country code. For testing purposes, setting SYNCEVOLUTION_PIM_EDS_SUBSTRING forces the usage of the traditional suffix match. This is used to test both the old and new flavor of testFilterStartupRefine. FilterStartupRefineSmart is the one which uses phone number matching. At the moment that fails to match phone numbers in EDS which do not contain a country code. Need to check whether that is meant to match. To reproduce: $ cd /tmp/; mkdir -p testing; cd testing; export XDG_CONFIG_HOME=/tmp/testing/temp-testpim/config XDG_DATA_HOME=/tmp/testing/temp-testpim/local/cache LANG=de_DE.utf-8; jhbuild run ~pohly/src/syncevolution/test/dbus-session.sh bash /tmp/testing$ rm -f *.log; SYNCEVOLUTION_NO_PIM_EDS_DIRECT=1 PATH=/home/pohly/work/syncevolution/src:$PATH ~pohly/src/syncevolution/src/dbus/server/pim/testpim.py -v TestContacts.testFilterStartupRefineSmart Beware!!! Setting LANG=de_DE.utf-8 before launching the dbus-daemon and thus evolution-addressbook-factory is important. This was not in previous instructions. Otherwise EDS may run with a different locale setting than the test. The updated testpim.py checks for this.
6th problem: locale changes must lead to re-evaluating cached values in the DB 1. Run evolution-addressbook-factory with LC_ALL=de_DE.utf-8. 2. Import the following contact: cat >test.vcf <<EOF BEGIN:VCARD VERSION:3.0 N:Zoo;Abraham TEL:1234 FN:Abraham Zoo END:VCARD EOF syncevolution --import test.vcf backend=evolution-contacts 3. Export it: syncevolution --export - backend=evolution-contacts BEGIN:VCARD VERSION:3.0 UID:pas-id-512735B900000000 X-EVOLUTION-FILE-AS:Zoo\, Abraham N:Zoo;Abraham TEL;X-EVOLUTION-E164=1234:1234 FN:Abraham Zoo REV:2013-02-22T09:09:13Z(1) END:VCARD 4. Run evolution-addressbook-factory with LC_ALL=en_US.utf-8 5. Export the previously imported contact: syncevolution --export - backend=evolution-contacts BEGIN:VCARD VERSION:3.0 UID:pas-id-512735B900000000 X-EVOLUTION-FILE-AS:Zoo\, Abraham N:Zoo;Abraham TEL;X-EVOLUTION-E164=1234:1234 FN:Abraham Zoo REV:2013-02-22T09:09:13Z(1) END:VCARD Same as before. 6. Import again, with a different name: cat >test2.vcf <<EOF BEGIN:VCARD VERSION:3.0 N:Zoo2;Abraham TEL:1234 FN:Abraham Zoo2 END:VCARD EOF syncevolution --import test2.vcf backend=evolution-contacts 7. Export both and compare: syncevolution --export - backend=evolution-contacts BEGIN:VCARD VERSION:3.0 UID:pas-id-512735B900000000 X-EVOLUTION-FILE-AS:Zoo\, Abraham N:Zoo;Abraham TEL;X-EVOLUTION-E164=1234:1234 FN:Abraham Zoo REV:2013-02-22T09:09:13Z(1) END:VCARD BEGIN:VCARD VERSION:3.0 UID:pas-id-5127360800000000 X-EVOLUTION-FILE-AS:Zoo2\, Abraham N:Zoo2;Abraham TEL;X-EVOLUTION-E164=234:1234 FN:Abraham Zoo2 REV:2013-02-22T09:10:32Z(1) END:VCARD Note that the libphonenumber interprets 1234 differently, depending on the current locale. If in the US, the 1 must get interpreted as some kind of dialing prefix, leading to a different national number. That means when switching to or from the US (or switching locales in general), all numbers must be re-parsed to ensure that the cached values are still correct. This is not happening, as can be seen in the output above. The two contacts should always have the same E164 parameter.
OK, so just for extra-clarity: "TEL;X-EVOLUTION-E164=234:1234" in the 2nd contact should be "TEL;X-EVOLUTION-E164=234:234" because it was parsed as a contact interpreted with US locale. Correct?
(In reply to comment #9) > OK, so just for extra-clarity: > > "TEL;X-EVOLUTION-E164=234:1234" in the 2nd contact should be > "TEL;X-EVOLUTION-E164=234:234" because it was parsed as a contact > interpreted with US locale. No. The part after the colon is always the original telephone number, which must not change. The part which must match the current locale is the parameter value (X-EVOLUTION-E164=234). So if the current locale is US, then X-EVOLUTION-E164=234 is correct. If it is DE, then X-EVOLUTION-E164=1234 is right. This applies to both test cases.
(In reply to comment #7) > 5th problem: after switching to E_BOOK_QUERY_EQUALS_PHONE_NUMBER with a E164 > caller ID as comparison value (includes country code!), contacts which have > no country code don't match. It turns out that this is intentional. An API description patch for EDS is in the works. In SyncEvolution, I now use EQUALS_NATIONAL_PHONE_NUMBER, which matches contacts without explicit country code if the national part is the same. As intended, it does not match if there is a non-matching explicit country code. The issue however is not resolved entirely, because when summaries are enabled, EQUALS_NATIONAL_PHONE_NUMBER returns different results than without. Therefore SyncEvolution does *not* use extended summaries at the moment. It has to do that to improve performance of the direct phone number lookup.
Things should work now according to those tests. https://git.gnome.org/browse/evolution-data-server/tree/tests/libebook/client/test-client-change-country-code.c?h=openismus-work&id=8051c2a1802d4b04cbea0409ff026f05598ce146#n331 https://git.gnome.org/browse/evolution-data-server/tree/tests/libebook/client/test-client-custom-summary.c?h=openismus-work&id=8051c2a1802d4b04cbea0409ff026f05598ce146#n429
(In reply to comment #12) > Things should work now according to those tests. > > https://git.gnome.org/browse/evolution-data-server/tree/tests/libebook/ > client/test-client-change-country-code.c?h=openismus- > work&id=8051c2a1802d4b04cbea0409ff026f05598ce146#n331 > > https://git.gnome.org/browse/evolution-data-server/tree/tests/libebook/ > client/test-client-custom-summary.c?h=openismus- > work&id=8051c2a1802d4b04cbea0409ff026f05598ce146#n429 These are the relevant commits: 7748a56 sqlitedb: Only create indexes after introspection 606b360 sqlitedb: Use proper length of default country code 74e3f1e sqlitedb: Fix another issue phonenumber matching issue 3443ed1 sqlitedb: Assing country-code to address summary They reestablish rewriting the summary when the local changes. This solution still needs to be upstreamed.
(In reply to comment #13) > (In reply to comment #12) > > Things should work now according to those tests. > > > > https://git.gnome.org/browse/evolution-data-server/tree/tests/libebook/ > > client/test-client-change-country-code.c?h=openismus- > > work&id=8051c2a1802d4b04cbea0409ff026f05598ce146#n331 > > > > https://git.gnome.org/browse/evolution-data-server/tree/tests/libebook/ > > client/test-client-custom-summary.c?h=openismus- > > work&id=8051c2a1802d4b04cbea0409ff026f05598ce146#n429 > > These are the relevant commits: > 7748a56 sqlitedb: Only create indexes after introspection > 606b360 sqlitedb: Use proper length of default country code > 74e3f1e sqlitedb: Fix another issue phonenumber matching issue > 3443ed1 sqlitedb: Assing country-code to address summary > > They reestablish rewriting the summary when the local changes. This solution > still needs to be upstreamed. Again, we've still had issues convincing Milan that the addressbook data needs to be re-written on a locale change (or at least for a country code change), that still needs to be upstreamed. Regarding some different behaviour that was occurring when matching phone numbers in the SQLite vs matching directly with the vCard data, we did catch that issue and it was fixed in this commit: commit a3526c06166073e3c96a4bfef6a294b5cc71f78b Author: Mathias Hasselmann <mathias@openismus.com> Date: Fri Mar 1 14:05:28 2013 +0100 sqlitedb: Improve national phone number matching The sqlitedb backend and e_phone_number_compare() showed different behavior for a few cases of national and short number matching. This syncs the behavior.
(In reply to comment #14) > (In reply to comment #13) > > (In reply to comment #12) > > > Things should work now according to those tests. > > > > > > https://git.gnome.org/browse/evolution-data-server/tree/tests/libebook/ > > > client/test-client-change-country-code.c?h=openismus- > > > work&id=8051c2a1802d4b04cbea0409ff026f05598ce146#n331 > > > > > > https://git.gnome.org/browse/evolution-data-server/tree/tests/libebook/ > > > client/test-client-custom-summary.c?h=openismus- > > > work&id=8051c2a1802d4b04cbea0409ff026f05598ce146#n429 > > > > These are the relevant commits: > > 7748a56 sqlitedb: Only create indexes after introspection > > 606b360 sqlitedb: Use proper length of default country code > > 74e3f1e sqlitedb: Fix another issue phonenumber matching issue > > 3443ed1 sqlitedb: Assing country-code to address summary > > > > They reestablish rewriting the summary when the local changes. This solution > > still needs to be upstreamed. > > Again, we've still had issues convincing Milan that the addressbook data > needs to be re-written on a locale change (or at least for a country code > change), that still needs to be upstreamed. I think I managed to convince Milan that rewrites on "preferred location" change are necessary. From IRC (sorry, don't know the date): (12:28:30 PM) mcrha: yes, and no. I believe the index rebuild is not needed (12:29:17 PM) tristan left the room (quit: Read error: 148 (No route to host)). (12:30:14 PM) heroux [~heroux@5070823C.static.ziggozakelijk.nl] entered the room. (12:30:35 PM) kendy_ [~kendy@nue-nat.emea.novell.com] entered the room. (12:30:37 PM) mcrha: to clarify that we talk about the same: the problem discussed here is that the phone number indexes doesn't work correctly for phone numbers without country codes, because eds currently stores guessed country codes there (based on current locale or whatever), while keeping the contact as is? (12:31:43 PM) pohly: mcrha: no. EDS does not store guessed country codes in the index. (12:31:57 PM) pohly: Let's take an example. hasselmm_ heroux heroux_ (12:33:16 PM) pohly: For the US, libphonenumber parses 1234 as local number 234 because 1 is a local dialing prefix. I think hasselmm_ was wrong earlier when he said that 1 is turned into the +1 country code. (12:33:39 PM) pohly: In other countries, 1234 is used as it is as the local number. (12:33:57 PM) xtian is now known as xtian|munch (12:33:57 PM) pohly: In both cases, the country code is explicitly marked as unset by libphonenumber. (12:34:19 PM) pohly: In EDS, that means that the country code for that number is unset in the index. (12:34:36 PM) pohly: But the local part in the index is different: 234 in the US, 1234 elsewhere. (12:35:10 PM) heroux_ left the room (quit: Ping timeout: 600 seconds). (12:35:35 PM) pohly: When the lookup happens when outside the US, it is done with 1234 as local part. This fails when the index was created in the US. (12:37:31 PM) pohly: Suppose the number really is a local German number 1234. Should the lookup fail just because I imported the contact while the country was set to US? No. (12:38:14 PM) pohly: Therefore EDS needs to re-evaluate such ambiguous numbers when the country changes. (12:39:42 PM) mcrha: right, it makes me feel like it being a proof why using application current locale for this is wrong. (12:39:46 PM) kendy left the room (quit: Ping timeout: 600 seconds). (12:40:21 PM) pohly: mcrha: what's the alternative? Use a fixed country that cannot be changed by the user? (12:40:44 PM) kendy_ left the room (quit: Ping timeout: 600 seconds). (12:41:00 PM) mcrha: No. As I mentioned above, a user visible option. (12:41:21 PM) pohly: And what if the user changes that option? Same problem -> index becomes invalid. (12:41:58 PM) pohly: Or are you saying that changing that option should trigger a rewrite the DB, with that code living where the option is handled? (12:42:05 PM) mcrha: then you can rebuild the index (which will most likely cause writes in DRA, maybe not) (12:43:01 PM) pohly: But you would rebuild in EDS itself, right? (12:43:49 PM) mcrha: pohly, you convinced me that rewrite might be done. I want an explicit option for default country code for phone numbers, thus it'll not be guessed from nowhere. (12:43:56 PM) mcrha: yes, in eds (12:44:36 PM) pohly: Okay, so we agree that rewriting is necessary on country changes. Now we can focus on where that information is supposed to come from when EDS starts. He likes to see some more explicit code for detecting the country. Deriving it from the locale is just a heuristic. I agree with him on that, but I am not sure what the right solution is. Milan and I seem to disagree on that: (12:44:36 PM) pohly: Okay, so we agree that rewriting is necessary on country changes. Now we can focus on where that information is supposed to come from when EDS starts. (12:45:14 PM) pohly: It can't be in Evolution, because Evolution might not be running and EDS should not depend on settings of one particular GUI. (12:45:56 PM) mcrha: where you get the value for that explicit option is up to you, being it a GSettings key, a phone-provided country, system setting country (like system timezone?), or whatever is not that important, just do not use current locale, which doesn't tell you where the person actually is (12:46:40 PM) mcrha: right, for eds on linux it might be eds' GSettings key. (12:46:53 PM) mcrha: "on linux desktop" it is (12:47:14 PM) pohly: And who sets that key? (12:47:35 PM) mcrha: anyone? (12:48:07 PM) kendy_ [~kendy@nat.scz.novell.com] entered the room. (12:48:25 PM) pohly: Let's ignore other desktop systems and look at just GNOME. Which GUI component in GNOME would set the value? (12:48:27 PM) mcrha: current locale is also set by anyone (12:48:47 PM) mcrha: evolution will have that option available for sure (12:49:10 PM) mcrha: if others will want to touch it, then they can too. (12:49:37 PM) pohly: I doubt that you'll be able to convince other software developers to write an EDS specific GSettings key. (12:49:53 PM) pohly: What we need is a freedesktop.org standard for setting the current location. (12:50:18 PM) mcrha: I'm not forcing them, they can do it, but it's not mandatory (12:50:19 PM) Jignesh [~Jignesh@14.97.40.71] entered the room. (12:51:16 PM) mcrha: if I would want to do that simple, then I say to read the key value on start of the addressbook factory and work with it all its runtime. As it influences only indexes, then it should not be a problem, right? (12:52:04 PM) mcrha: pohly, freedesktop.org would be interesting, but as long as it's for eds only... Maybe once other parts of the system will need it (12:52:53 PM) mcrha: pohly, hmm, is this "only" about indexes? (12:53:00 PM) {srinidhi} [~srinidhi@117.202.64.228] entered the room. (12:53:09 PM) pohly: I'm not convinced. Depending on a setting that is likely to be unset unless the user explicitly sets it in Evolution (and knows about it!) seems to me a step backwards compared to guessing based on the locale, which is guaranteed to be set. I'm merely pragmatic here. IMHO we should have an explicit setting and the locale based approach as fallback. (12:53:35 PM) pohly: Indices and lookups. (12:54:10 PM) mcrha: right, index and lookup (12:55:19 PM) mcrha: pohly, will the index work properly if a US number 1234 will be renormalized in Geramy in indexes, and you'll search for 234? (12:55:35 PM) mcrha: *for Germany (12:57:58 PM) mcrha: you said that US:1234 is turned into 1|234, thus if I search either for 1234 or 234 I still get the same contact, but the Germany changes 1234 to |1234, thus searching for 234 will not work "suddenly"? (12:58:38 PM) pohly: mcrha: there is no "US:1234" - that would be "+1 1234". (12:58:51 PM) pohly: There is just a contact with TEL:1234. (12:59:08 PM) pohly: Where EDS doesn't know whether that number is a US or German number. (12:59:11 PM) mcrha: it was meant as my notation for "1234" with "US as the default country code" (01:00:21 PM) pohly: Correct, in the US, searching for 234 would match TEL:1234 and TEL:234. In Germany, 234 would not match TEL:1234. (01:00:34 PM) pohly: Or rather, should. (01:01:15 PM) mcrha: That would be very confusing for users, no? (01:01:37 PM) pohly: Why? (01:02:10 PM) kjetilho: god, I hate how that works most places. so many places assume that your phone number is located in the same country as your street address (01:02:15 PM) mcrha: if I want to search for 234 in evolution's UI, then I expect consistent results (01:02:32 PM) pohly: You are talking about a substring search. That still works as before. (01:02:36 PM) kjetilho: so you can't use a Swedish phone number if you live in Denmark, since +45 is implied (01:03:02 PM) pohly: The semantic search only applies when the search strings is known to be a valid phone number. (01:04:37 PM) pohly: This difference is important! The semantic search is primarily meant to be used for caller IDs, which always have an explicit country code. (01:05:24 PM) Jignesh left the room (quit: Remote closed the connection). (01:05:32 PM) pohly: So the search in Germany would be for +491234 or +49234. The former matches TEL:1234, but not TEL:234, and vice versa. (01:06:40 PM) pohly: When the user enters 234 with the intention to dial the matching number from his address book, then a substring search must be done and yes, that needs to find both TEL:1234 and TEL:234. (01:08:00 PM) pohly: Strictly speaking, it shouldn't be an exact substring match as currently done by EDS. A better solution would find both "TEL:12 34" and "TEL:1234" when searching for "234" - we don't have that in EDS. (01:10:20 PM) mcrha: ok pohly, we agreed that a) index rebuild is needed due to contacts without country code when the default country code changes; b) the default country code may come from trustful source, either some explicit setting or, for your phone usecase, from phone (01:11:26 PM) mcrha: b) Using a default country code is wrong, because one may miss contacts on lookup in certain cases? (01:11:45 PM) mcrha: err, c) Using... (01:12:34 PM) pohly: Please be more specific about case c). Using the default country code for what? (01:12:54 PM) pohly: We agree on a) and b). (01:13:26 PM) mcrha: for the setting, as you wanted to fallback based on current locale, if it's not set, while I would add a default value (other than empty string) (01:13:51 PM) mcrha: thus the setting is always set (01:15:18 PM) pohly: Then we disagree about c. I think we need a fallback for cases where the country is not set explicitly. (01:16:51 PM) pohly: Using the current locale is more likely to be right than falling back to a fixed value. (01:17:08 PM) pohly: Or refusing to execute semantic searches. (01:17:18 PM) pohly: That would be the alternative. (01:18:15 PM) pohly: We do agree about c in the sense that a wrong guess can lead to unexpected results. (01:18:39 PM) pohly: So it certainly would be desirable to have a more visible explicit setting that the user is aware of. (01:19:21 PM) pohly: Need something to eat - will be back later. (01:20:32 PM) mcrha: same here, feeding time :) (01:21:07 PM) mcrha: by the way, for me the locale is wrong, I sit in Czech Republic, but my locale is en_US. (01:22:07 PM) pohly: mcrha: my setup is similar. But we are power users and know better than entering ambiguous telephone numbers into our address books, right? (01:22:34 PM) mcrha: One can guess less when examining the vCard more thoroughly, like pairing home phone with home address' country, similar for work phone & address, but it doesn't work always, and is a minor improvement (01:22:48 PM) pohly: At least I switched to fully qualified numbers years ago, because I wanted my address book to work everywhere. (01:23:20 PM) mcrha: hehe, I enter 1234 or "home-phone" quite often into Contacts in evo :) My phone is full of "no-country-code" phone numbers (01:23:37 PM) pohly: Well, man up and fix that ;-) (01:23:46 PM) mcrha: :) (01:24:42 PM) mcrha: it seems the result of our chat will be "the best effort" approach, which can fail in certain cases (01:24:46 PM) pohly: It's necessary when you sync to a phone and travel, because I guarantee you that the phone will not be able to dial these local numbers from abroad. (01:25:17 PM) pohly: mcrha: yes, it's all about heuristics and doing what is right most of the time. (01:25:27 PM) mcrha: yeah, I agree (and do not travel) :) (01:25:44 PM) pohly: So now I really need to go.... (01:25:51 PM) mcrha: sure, later
The phone numbers currently don't renormalize according to locale changes, but now there is a dynamic locale setting driven by systemd's org.freedesktop.locale1 interface. Add local D-Bus wrapper to listen to locale changes: https://git.gnome.org/browse/evolution-data-server/commit/?h=openismus-work-3-8&id=5706361d082a8eb33a29d6afaedaf9e71865e91f Add addressbook D-Bus locale property: https://git.gnome.org/browse/evolution-data-server/commit/?h=openismus-work-3-8&id=f48972a6e2ca03eb50aad2331bc62bdd5ae7e85c Add EBookBackend APIs to get/set the addressbook locale setting: https://git.gnome.org/browse/evolution-data-server/commit/?h=openismus-work-3-8&id=f44aa895cdecd97651dfa2a42717a012ed9607a5 Listen to org.freedesktop.locale1 locale change settings (systemd controls the setting): https://git.gnome.org/browse/evolution-data-server/commit/?h=openismus-work-3-8&id=2d287ca7d2ab45af28deb1847b9fdcad0626ec71 Store current locale setting in the SQLite, renormalize collation keys when locale changes: https://git.gnome.org/browse/evolution-data-server/commit/?h=openismus-work-3-8&id=87a2a86dd6a8fb17bcb08dae1366cef4774be3e3 EBookBackendFile, file backend implements set_locale() / get_locale(): https://git.gnome.org/browse/evolution-data-server/commit/?h=openismus-work-3-8&id=87efc5c00d952a0a17885cfce37ec62fd7b74bea EBookClient, add locale property, clients can read/watch the addressbook locale setting: https://git.gnome.org/browse/evolution-data-server/commit/?h=openismus-work-3-8&id=744a4d685e694547c0bc561e0a1be2be3cf6f474 Few other patches add tests to check sort order changes when the locale changes.
(In reply to comment #16) > The phone numbers currently don't renormalize according to locale changes, Wasn't that already implemented by Mathias earlier? See the commits in comment #13. Mathias wasn't very clear in his commit messages and the code itself was a bit mysterious, too (at least for me). Have these commits been dropped when moving to 3.8 as base for openismus-work? Or were they integrated into the 3.8 upstream release?
(In reply to comment #16) > Listen to org.freedesktop.locale1 locale change settings (systemd controls > the setting): > https://git.gnome.org/browse/evolution-data-server/commit/?h=openismus-work- > 3-8&id=2d287ca7d2ab45af28deb1847b9fdcad0626ec71 I think this code also needs to handle LC_ALL. See "man setlocale": LC_ALL overrides LC_COLLATE, which overrides LANG.
(In reply to comment #18) > (In reply to comment #16) > > Listen to org.freedesktop.locale1 locale change settings (systemd controls > > the setting): > > https://git.gnome.org/browse/evolution-data-server/commit/?h=openismus-work- > > 3-8&id=2d287ca7d2ab45af28deb1847b9fdcad0626ec71 > > I think this code also needs to handle LC_ALL. See "man setlocale": LC_ALL > overrides LC_COLLATE, which overrides LANG. At first sight (reading this comment) I thought I must have had it backwards... but I double checked and verified this and the current handling seems to be correct. The systemd definition of the 'Locale' property (on the org.freedesktop.locale1 interface) specifies a limited set of locale setting strings: http://www.freedesktop.org/wiki/Software/systemd/localed/ "The following strings are known: LANG=, LC_CTYPE=, LC_NUMERIC=, LC_TIME=, LC_COLLATE=, LC_MONETARY=, LC_MESSAGES=, LC_PAPER=, LC_NAME=, LC_ADDRESS=, LC_TELEPHONE=, LC_MEASUREMENT=, LC_IDENTIFICATION="
The current openismus-work-3-8 branch has code which reacts to locale changes: commit 2d287ca7d2ab45af28deb1847b9fdcad0626ec71 Author: Tristan Van Berkom <tristanvb@openismus.com> Date: Thu May 23 15:24:09 2013 +0900 EDataBook: Watch the system bus for locale notifications When org.freedesktop.locale1 is available, listen to changes in the LC_COLLATE locale and configure backends with locale changes using e_book_backend_set_locale(), notify property changes via the locale property on the addressbook D-Bus API. Also, load the backend's initially set locale as the locale property value until the org.freedesktop.locale1 D-Bus interface notifies us of a locale change on the system bus. However, it is (partially?) broken. From a test run involving a SyncEvolution test: ==3285== Command: /data/runtests/install/testing-amd64/evo-src-master/libexec//evolution-addressbook-factory --keep-r unning ==3285== Parent PID: 3281 ==3285== ==3285== Invalid read of size 8 ==3285== at 0x4E69054: data_book_locale_changed (e-data-book.c:2046) ==3285== by 0x4E691F5: data_book_localed_ready (e-data-book.c:2077) ==3285== by 0x90ED8D6: g_simple_async_result_complete (gsimpleasyncresult.c:777) ==3285== by 0x90ED9D8: complete_in_idle_cb (gsimpleasyncresult.c:789) ==3285== by 0x966D3D4: g_main_context_dispatch (gmain.c:3054) ==3285== by 0x966D717: g_main_context_iterate.isra.22 (gmain.c:3701) ==3285== by 0x966DB89: g_main_loop_run (gmain.c:3895) ==3285== by 0x52E051C: dbus_server_run_server (e-dbus-server.c:222) ==3285== by 0x10CCB7BB: ffi_call_unix64 (in /usr/lib/x86_64-linux-gnu/libffi.so.5.0.10) ==3285== by 0x10CCB236: ffi_call (in /usr/lib/x86_64-linux-gnu/libffi.so.5.0.10) ==3285== by 0x93E60BA: g_cclosure_marshal_generic_va (gclosure.c:1550) ==3285== by 0x93E53E6: _g_closure_invoke_va (gclosure.c:840) ==3285== by 0x93FDD45: g_signal_emit_valist (gsignal.c:3234) ==3285== by 0x93FE921: g_signal_emit (gsignal.c:3384) ==3285== by 0x52E07B9: e_dbus_server_run (e-dbus-server.c:414) ==3285== by 0x400E54: main (evolution-addressbook-factory.c:132) ==3285== Address 0x199b0be8 is 24 bytes inside a block of size 176 free'd ==3285== at 0x4C2A68C: free (vg_replace_malloc.c:446) ==3285== by 0x94050C4: g_type_free_instance (gtype.c:1962) ==3285== by 0x4E69488: op_unref (e-data-book.c:374) ==3285== by 0x4E6CB8F: operation_thread (e-data-book.c:754) ==3285== by 0x9691881: g_thread_pool_thread_proxy (gthreadpool.c:309) ==3285== by 0x9691044: g_thread_proxy (gthread.c:798) ==3285== by 0x9952E0D: start_thread (pthread_create.c:311) ==3285== by 0x9C4F9EC: clone (clone.S:113) ==3285== { <insert_a_suppression_name_here> Memcheck:Addr8 fun:data_book_locale_changed fun:data_book_localed_ready fun:g_simple_async_result_complete fun:complete_in_idle_cb fun:g_main_context_dispatch fun:g_main_context_iterate.isra.22 fun:g_main_loop_run fun:dbus_server_run_server fun:ffi_call_unix64 fun:ffi_call fun:g_cclosure_marshal_generic_va fun:_g_closure_invoke_va fun:g_signal_emit_valist fun:g_signal_emit fun:e_dbus_server_run fun:main } ==3285== Invalid read of size 8 ==3285== at 0x4E6905B: data_book_locale_changed (e-data-book.c:2046) ==3285== by 0x4E691F5: data_book_localed_ready (e-data-book.c:2077) ==3285== by 0x90ED8D6: g_simple_async_result_complete (gsimpleasyncresult.c:777) ==3285== by 0x90ED9D8: complete_in_idle_cb (gsimpleasyncresult.c:789) ==3285== by 0x966D3D4: g_main_context_dispatch (gmain.c:3054) ==3285== by 0x966D717: g_main_context_iterate.isra.22 (gmain.c:3701) ==3285== by 0x966DB89: g_main_loop_run (gmain.c:3895) ==3285== by 0x52E051C: dbus_server_run_server (e-dbus-server.c:222) ==3285== by 0x10CCB7BB: ffi_call_unix64 (in /usr/lib/x86_64-linux-gnu/libffi.so.5.0.10) ==3285== by 0x10CCB236: ffi_call (in /usr/lib/x86_64-linux-gnu/libffi.so.5.0.10) ==3285== by 0x93E60BA: g_cclosure_marshal_generic_va (gclosure.c:1550) ==3285== by 0x93E53E6: _g_closure_invoke_va (gclosure.c:840) ==3285== by 0x93FDD45: g_signal_emit_valist (gsignal.c:3234) ==3285== by 0x93FE921: g_signal_emit (gsignal.c:3384) ==3285== by 0x52E07B9: e_dbus_server_run (e-dbus-server.c:414) ==3285== by 0x400E54: main (evolution-addressbook-factory.c:132) ==3285== Address 0x8 is not stack'd, malloc'd or (recently) free'd ==3285== { <insert_a_suppression_name_here> Memcheck:Addr8 fun:data_book_locale_changed fun:data_book_localed_ready fun:g_simple_async_result_complete fun:complete_in_idle_cb fun:g_main_context_dispatch fun:g_main_context_iterate.isra.22 fun:g_main_loop_run fun:dbus_server_run_server fun:ffi_call_unix64 fun:ffi_call fun:g_cclosure_marshal_generic_va fun:_g_closure_invoke_va fun:g_signal_emit_valist fun:g_signal_emit fun:e_dbus_server_run fun:main } ==3285== ==3285== Process terminating with default action of signal 11 (SIGSEGV) ==3285== Access not within mapped region at address 0x8 ==3285== at 0x4E6905B: data_book_locale_changed (e-data-book.c:2046) ==3285== by 0x4E691F5: data_book_localed_ready (e-data-book.c:2077) ==3285== by 0x90ED8D6: g_simple_async_result_complete (gsimpleasyncresult.c:777) ==3285== by 0x90ED9D8: complete_in_idle_cb (gsimpleasyncresult.c:789) ==3285== by 0x966D3D4: g_main_context_dispatch (gmain.c:3054) ==3285== by 0x966D717: g_main_context_iterate.isra.22 (gmain.c:3701) ==3285== by 0x966DB89: g_main_loop_run (gmain.c:3895) ==3285== by 0x52E051C: dbus_server_run_server (e-dbus-server.c:222) ==3285== by 0x10CCB7BB: ffi_call_unix64 (in /usr/lib/x86_64-linux-gnu/libffi.so.5.0.10) ==3285== by 0x10CCB236: ffi_call (in /usr/lib/x86_64-linux-gnu/libffi.so.5.0.10) ==3285== by 0x93E60BA: g_cclosure_marshal_generic_va (gclosure.c:1550) ==3285== by 0x93E53E6: _g_closure_invoke_va (gclosure.c:840) ==3285== by 0x93FDD45: g_signal_emit_valist (gsignal.c:3234) ==3285== by 0x93FE921: g_signal_emit (gsignal.c:3384) ==3285== by 0x52E07B9: e_dbus_server_run (e-dbus-server.c:414) ==3285== by 0x400E54: main (evolution-addressbook-factory.c:132) This happens with the TestContact.testLocaledPhone test from SyncEvolution, modified to rely on EDS: diff --git a/src/dbus/server/pim/testpim.py b/src/dbus/server/pim/testpim.py index a2e1f71..2315bcd 100755 --- a/src/dbus/server/pim/testpim.py +++ b/src/dbus/server/pim/testpim.py @@ -58,7 +58,7 @@ if testFolder not in sys.path: # Rely on the glib/gobject compatibility import code in test-dbus.py. from testdbus import glib, gobject -from testdbus import DBusUtil, timeout, property, usingValgrind, xdg_root, bus, logging, NullLogging, loop +from testdbus import DBusUtil, timeout, Timeout, property, usingValgrind, xdg_root, bus, logging, NullLogging, loop import testdbus def timeFunction(func, *args1, **args2): @@ -2677,15 +2677,21 @@ END:VCARD raise Exception('%s:\n%s' % (msg, repr(ex))), None, info[2] else: raise + finally: + daemon.remove_from_connection() @timeout(60) - # Must disable usage of pre-computed phone numbers from EDS, because we can't tell EDS - # when locale is meant to change. - @property("ENV", "LANG=en_US.UTF-8 SYNCEVOLUTION_PIM_EDS_NO_E164=1") + @property("ENV", "LANG=en_US.UTF-8") def testLocaledPhone(self): # Parsing of 1234-5 depends on locale: US drops the 1 from 1234 # Germany (and other countries) don't. Use that to match (or not match) # a contact. + + daemon = localed.Localed() + # Give EDS some time to notice the new daemon and it's en_US setting. + Timeout.addTimeout(5, loop.quit) + loop.run() + testcases = (('Doe', '''BEGIN:VCARD VERSION:3.0 FN:Doe @@ -2698,7 +2704,6 @@ END:VCARD view = self.doFilter(testcases, (([['phone', '+12345']], ('Doe',)),)) - daemon = localed.Localed() msg = None try: # Contact no longer matched because it's phone number normalization @@ -2723,6 +2728,8 @@ END:VCARD raise Exception('%s:\n%s' % (msg, repr(ex))), None, info[2] else: raise + finally: + daemon.remove_from_connection() # Not supported correctly by ICU? # See icu-support "Subject: Austrian phone book sorting" There might be simpler ways to trigger the problem, like running evolution-addressbook-factory under valgrind and then manually changing the localed settings.
The EDS locale change crash is tracked upstream at https://bugzilla.gnome.org/show_bug.cgi?id=723276
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/SyncEvolution/syncevolution/issues/45.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.