After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 653623 - Would like a set of standard "type" parameters for UrlDetails
Would like a set of standard "type" parameters for UrlDetails
Status: RESOLVED FIXED
Product: folks
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: folks-0.6.4
Assigned To: folks-maint
folks-maint
Depends on:
Blocks:
 
 
Reported: 2011-06-29 09:07 UTC by Alexander Larsson
Modified: 2011-09-18 18:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add standard URL type values as consts to UrlFieldDetails (27.29 KB, patch)
2011-09-16 22:37 UTC, Philip Withnall
reviewed Details | Review
Add standard URL type values as consts to UrlFieldDetails (updated) (29.29 KB, patch)
2011-09-18 14:16 UTC, Philip Withnall
committed Details | Review

Description Alexander Larsson 2011-06-29 09:07:45 UTC
The current eds backend gives me empty parameters for the UrlDetails fields, but it would be nice to have at least a few standard types so that these can be displayed nicely.

eds currently store 4 uris, and they get mapped to:

Homepage:
URL:http://www.lysator.liu.se/~alla
blog:
X-EVOLUTION-BLOG-URL:http://blog
calendar:
CALURI:http://calendar
free/busy:
FBURL:http://free-busy
Video chat:
X-EVOLUTION-VIDEO-URL:http://video-chat

Google contacts allows the predefined types: "Profile", "Blog", "Home Page" and "Work", but also lets you give a custom label string.
Google doesn't use vcard as a native store, but you can export to vcard for importing into the mac address book. You then get:

item2.URL:http\://profil
item2.X-ABLabel:PROFILE
item3.URL:http\://blog
item3.X-ABLabel:BLOG
item4.URL:http\://homepage
item4.X-ABLabel:_$!<HomePage>!$_
URL;TYPE=WORK:http\://work
item5.URL:http\://custom.com
item5.X-ABLabel:CustomLabel

In this, only TYPE=WORK is standard, and the rest use the apple X-ABLabel extension that is similar to the x-google-label extension that we added, but that also supports e.g. _$!<HomePage>!$_ to mean that something is an internal string that should be translated in the ui.

I'm not sure what the best mapping of these things are. We could just declare some standard types like TYPE=HOMEPAGE, TYPE=BLOG and then use x-google-label for the rest. Or we could use specific extensions x-gnome-foo for everything...
Comment 1 Travis Reitter 2011-07-05 17:34:50 UTC
(In reply to comment #0)

> Google contacts allows the predefined types: "Profile", "Blog", "Home Page" and
> "Work", but also lets you give a custom label string.
> Google doesn't use vcard as a native store, but you can export to vcard for
> importing into the mac address book. You then get:
> 
> item2.URL:http\://profil
> item2.X-ABLabel:PROFILE
> item3.URL:http\://blog
> item3.X-ABLabel:BLOG
> item4.URL:http\://homepage
> item4.X-ABLabel:_$!<HomePage>!$_
> URL;TYPE=WORK:http\://work
> item5.URL:http\://custom.com
> item5.X-ABLabel:CustomLabel

This "objectified" vCard notation is a bit odd - I've never seen it before, and I'm not sure it's valid. I can't find anything similar in the vCard RFCs. It would be nice, though, since the LABEL field has traditionally had the problem that it's not strictly tied to a specific ADR. Eg:

ADR;TYPE=WORK:;;100 Waters Edge;Baytown;LA;30314;United States of America
LABEL;TYPE=WORK:100 Waters Edge\nBaytown, LA 30314\nUnited States of America

That works OK if you've only got one pair of ADR and LABEL fields with TYPE=WORK, but there's no guarantee of that constraint. And I don't believe attribute ordering is ever guaranteed or considered (either in the RFCs or in implementations).

> In this, only TYPE=WORK is standard, and the rest use the apple X-ABLabel
> extension that is similar to the x-google-label extension that we added, but
> that also supports e.g. _$!<HomePage>!$_ to mean that something is an internal
> string that should be translated in the ui.
> 
> I'm not sure what the best mapping of these things are. We could just declare
> some standard types like TYPE=HOMEPAGE, TYPE=BLOG and then use x-google-label
> for the rest. Or we could use specific extensions x-gnome-foo for everything...

My vote would be for re-using whatever Apple and Google are using as much as possible. The naming might not be ideal, but it's worth being compatible.
Comment 2 Alexander Larsson 2011-07-06 19:19:53 UTC
The notation seems to be vcard grouping, per section 3.3. of  http://tools.ietf.org/html/draft-ietf-vcarddav-vcardrev-19 :

   The group construct is used to group related properties together.
   The group name is a syntactic convention used to indicate that all
   property names prefaced with the same group name SHOULD be grouped
   together when displayed by an application.  It has no other
   significance.  Implementations that do not understand or support
   grouping MAY simply strip off any text before a "." to the left of
   the type name and present the types and values as normal.

> My vote would be for re-using whatever Apple and Google are using as much as
> possible. The naming might not be ideal, but it's worth being compatible.

Don't be fooled by the x-google-label name. This is a pure gnome construct that libgdata uses to map things expressed in the native google contacts db as vcards. We should probably switch this to either be x-gnome-label or use the apple construct.
Comment 3 Travis Reitter 2011-08-23 16:40:53 UTC
(Setting target bugs for Folks 0.6.1. Search for this phrase to deal with bug mail spam.)
Comment 4 Raul Gutierrez Segales 2011-08-29 12:46:41 UTC
Didn't make this release; punting to 0.6.2.
Comment 5 Travis Reitter 2011-09-05 16:49:24 UTC
(Punting bugs to 0.6.4)
Comment 6 Philip Withnall 2011-09-16 22:37:21 UTC
Created attachment 196780 [details] [review]
Add standard URL type values as consts to UrlFieldDetails

https://www.gitorious.org/folks/folks/commits/653623-url-types

Pretty rough patch to do this.
Comment 7 Travis Reitter 2011-09-18 01:43:03 UTC
Review of attachment 196780 [details] [review]:

/* Keep in sync with Edsf.Persona.url_properties */

Should be _url_properties


-                    affl_info.affl_tracker_id, "website");
+                    affl_info.affl_tracker_id,
+                    UrlFieldDetails.PARAM_TYPE_HOMEPAGE);

-                else if (type_p.contains ("website"))
+                else if (type_p.contains (UrlFieldDetails.PARAM_TYPE_HOMEPAGE))

These change the string from "website" to "homepage". Is that intentional? Do we "own" the value of this string in the context of Tracker contacts?


+   * Parameter value for URLs for the contact's personal or professional FTP
+   * server.

A part of me just died. What inspired this value?


Are we sure that we always do case-insensitive comparisons between the vCard parameters and their values? It seems like an OK strategy, as long as we're consistent with it. As far as I can recall, parameter names should always be treated case-insensitively and parameter values usually can (certainly for the well-known values).


Other than that, this looks good.
Comment 8 Philip Withnall 2011-09-18 14:16:43 UTC
Created attachment 196880 [details] [review]
Add standard URL type values as consts to UrlFieldDetails (updated)

https://www.gitorious.org/folks/folks/commits/653623-url-types-rebase1

Updated to not spam PARAM_TYPE_OTHER into URLs coming out of the Tracker backend. I also consolidated the code which uses Edsf.Persona._url_properties so that we don't have to keep things in sync with it.
Comment 9 Philip Withnall 2011-09-18 14:20:49 UTC
(In reply to comment #7)
> Review of attachment 196780 [details] [review]:
> 
> /* Keep in sync with Edsf.Persona.url_properties */
> 
> Should be _url_properties

I've rewritten that code so that it no longer needs keeping in sync.

> -                    affl_info.affl_tracker_id, "website");
> +                    affl_info.affl_tracker_id,
> +                    UrlFieldDetails.PARAM_TYPE_HOMEPAGE);
> 
> -                else if (type_p.contains ("website"))
> +                else if (type_p.contains
> (UrlFieldDetails.PARAM_TYPE_HOMEPAGE))
> 
> These change the string from "website" to "homepage". Is that intentional? Do
> we "own" the value of this string in the context of Tracker contacts?

We do, since it's the TYPE value which we use in UrlFieldDetails. As long as we consistently map PARAM_TYPE_HOMEPAGE to/from NCO_WEBSITE, etc., we should be fine.

> +   * Parameter value for URLs for the contact's personal or professional FTP
> +   * server.
> 
> A part of me just died. What inspired this value?

The values are copied from attachment #196700 [details] from bug #659079, since they're what Alex seems to have settled on to use in EDS' vCards. We probably want to use the same values as EDS, as it makes everything a lot simpler and less confusing.

In turn, the values which Alex has used are probably based on the ones used in the Google Contacts API here: http://code.google.com/apis/contacts/docs/3.0/reference.html#gcWebsite.

I don't see a problem with FTP, even though nobody will ever use it. It's probably better to have too many (distinct) well-defined values than to have too few, as long as they all make sense in some context.

> Are we sure that we always do case-insensitive comparisons between the vCard
> parameters and their values? It seems like an OK strategy, as long as we're
> consistent with it. As far as I can recall, parameter names should always be
> treated case-insensitively and parameter values usually can (certainly for the
> well-known values).

Reasonably sure, but not absolutely sure. I've made a few more comparisons case-insensitive in the updated patch.
Comment 10 Philip Withnall 2011-09-18 16:56:45 UTC
Comment on attachment 196880 [details] [review]
Add standard URL type values as consts to UrlFieldDetails (updated)

Committed with a few additional changes to add ‘X-’ prefixes to the non-standard TYPE values (as per the patch which I eventually committed for bug #659079).

commit 393faf239d2486f5be2b611ddf875f1c72378666
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Fri Sep 16 23:34:39 2011 +0100

    Bug 653623 — Would like a set of standard "type" parameters for UrlDetails
    
    Add some consts for TYPE and some common values for it for UrlFieldDetails.
    
    Closes: bgo#653623

 NEWS                                        |    5 ++
 backends/eds/lib/edsf-persona-store.vala    |   70 ++++++++++++--------------
 backends/eds/lib/edsf-persona.vala          |   36 +++++++++++++-
 backends/tracker/lib/trf-persona-store.vala |   19 +++++---
 backends/tracker/lib/trf-persona.vala       |   24 +++++++---
 folks/abstract-field-details.vala           |   44 ++++++++++++++++-
 folks/url-details.vala                      |   37 ++++++++++++++
 tests/eds/add-persona.vala                  |    9 ++--
 tests/eds/email-details.vala                |    4 +-
 tests/eds/phone-details.vala                |    9 ++--
 tests/eds/postal-address-details.vala       |    5 +-
 tests/eds/set-emails.vala                   |    3 +-
 tests/eds/set-phones.vala                   |    3 +-
 tests/eds/set-postal-addresses.vala         |    6 ++-
 tests/eds/set-properties-race.vala          |    6 ++-
 tests/eds/set-urls.vala                     |    4 +-
 tests/tracker/set-urls.vala                 |   45 ++++++++++-------
 17 files changed, 233 insertions(+), 96 deletions(-)
Comment 11 Travis Reitter 2011-09-18 18:30:43 UTC
(In reply to comment #9)

> > +   * Parameter value for URLs for the contact's personal or professional FTP
> > +   * server.
> > 
> > A part of me just died. What inspired this value?
> 
> The values are copied from attachment #196700 [details] from bug #659079, since they're
> what Alex seems to have settled on to use in EDS' vCards. We probably want to
> use the same values as EDS, as it makes everything a lot simpler and less
> confusing.
> 
> In turn, the values which Alex has used are probably based on the ones used in
> the Google Contacts API here:
> http://code.google.com/apis/contacts/docs/3.0/reference.html#gcWebsite.
> 
> I don't see a problem with FTP, even though nobody will ever use it. It's
> probably better to have too many (distinct) well-defined values than to have
> too few, as long as they all make sense in some context.

Sure, it makes sense for completeness (as long as we don't have a long list of never-used values).
Comment 12 Travis Reitter 2011-09-18 18:31:53 UTC
(In reply to comment #10)
> (From update of attachment 196880 [details] [review])
> Committed with a few additional changes to add ‘X-’ prefixes to the
> non-standard TYPE values (as per the patch which I eventually committed for bug
> #659079).
> 
> commit 393faf239d2486f5be2b611ddf875f1c72378666
> Author: Philip Withnall <philip@tecnocode.co.uk>
> Date:   Fri Sep 16 23:34:39 2011 +0100
> 
>     Bug 653623 — Would like a set of standard "type" parameters for UrlDetails
> 
>     Add some consts for TYPE and some common values for it for UrlFieldDetails.
> 
>     Closes: bgo#653623
> 
>  NEWS                                        |    5 ++
>  backends/eds/lib/edsf-persona-store.vala    |   70 ++++++++++++--------------
>  backends/eds/lib/edsf-persona.vala          |   36 +++++++++++++-
>  backends/tracker/lib/trf-persona-store.vala |   19 +++++---
>  backends/tracker/lib/trf-persona.vala       |   24 +++++++---
>  folks/abstract-field-details.vala           |   44 ++++++++++++++++-
>  folks/url-details.vala                      |   37 ++++++++++++++
>  tests/eds/add-persona.vala                  |    9 ++--
>  tests/eds/email-details.vala                |    4 +-
>  tests/eds/phone-details.vala                |    9 ++--
>  tests/eds/postal-address-details.vala       |    5 +-
>  tests/eds/set-emails.vala                   |    3 +-
>  tests/eds/set-phones.vala                   |    3 +-
>  tests/eds/set-postal-addresses.vala         |    6 ++-
>  tests/eds/set-properties-race.vala          |    6 ++-
>  tests/eds/set-urls.vala                     |    4 +-
>  tests/tracker/set-urls.vala                 |   45 ++++++++++-------
>  17 files changed, 233 insertions(+), 96 deletions(-)

This patch looks even better, so nice work.