On Fri, 2014-05-02 at 11:59 +0200, Lukas Zeller wrote:
> Hello Patrick,
> 
> On 02.05.2014, at 10:38, Patrick Ohly <[email protected]> wrote:
> 
> >> What do you mean by "independent" properties?
> > 
> > For example, ADR and TEL are independent in the sense that their values
> > are stored in different field arrays. Sharing the LABEL field for the
> > new parameter creates a dependency (or risk of collision) that did not
> > exist before.
> 
> But how can this work at all? The index in the field arrays is what
> relates the components (e.g. TEL, TEL_FLAGS and LABEL) to each other.
> 
> This would mean all of the field arrays would need to have rows for
> the *sum* of all the TEL+ADR+EMAIL+URL+xxx, with most rows empty, to
> be able to share a single LABEL array.
> 
> Now I see why you want to check the availability of LABEL row even if 
> assigned via a property parameter.
> 
> Hmmm - indeed this might work, at the expense of a lot of empty
> elements in the actual property field arrays.

Exactly, that's what I need and get with the patch.

I suspect that a groupfield would cause less empty elements if (and only
if) no groups are used, because then values from different properties
can use the same UNASSIGNED entry in the group field. But I haven't
checked whether that's really how it works; it is not very relevant with
Google because Google uses many group tags, even if the label is just
"Other".

If different properties end up sharing the same UNASSIGNED group tag,
then adding labels is harder in scripts (must move entries before
setting the tag), but that's not necessary at the moment.

>  I'm just not sure right now if the generator part of MimeProfile is
> prepared for properly skipping entirely empty rows to avoid generating
> empty property lines in the vCard output. But that would be fixable as
> well.

That part already works.

> > That's because the code which checks where to store the ADR value does
> > not check whether the LABEL at the position is available.
> > 
> > There's another problem:
> > 
> > TEL:123456789
> > ADR;X-ABLabel=labelAdr:xxx
> > 
> > A simplistic check "LABEL[0] empty" will accept position #0 for ADR. But
> > doing so will add the labelAdr also to the TEL, which was previously
> > created at position #0.
> > 
> > That means that the check has to be "LABEL[0] does not exist".
> 
> Yes. Fortunately, TItemField differentiates between empty and unassigned; the 
> check needs to be for the latter.

Unassigned is not good enough. If the LABEL array gets extended at the
end (for example, because the third ADR had a X-ABLabel parameter), then
I have unassigned entries at the beginning of the array which must not
be used, because there are already other properties associated with
them.

Here's the patch:

diff --git a/src/sysync/mimedirprofile.cpp b/src/sysync/mimedirprofile.cpp
index 5b0ecc7..3699ad0 100644
--- a/src/sysync/mimedirprofile.cpp
+++ b/src/sysync/mimedirprofile.cpp
@@ -4154,6 +4154,35 @@ bool TMimeDirProfileHandler::parseProperty(
                   else
                     dostore = true; // at least one field exists, we might 
store
                 }
+                // - check if fields used for parameters are empty
+                const TParameterDefinition *paramP = aPropP->parameterDefs;
+                while (paramP) {
+                  if (mimeModeMatch(paramP->modeDependency)
+#ifndef NO_REMOTE_RULES
+                      && (!paramP->ruleDependency || 
isActiveRule(paramP->ruleDependency))
+#endif
+                      ) {
+                    if (paramP->convdef.fieldid==FID_NOT_SUPPORTED)
+                      continue; // no field, no need to check it
+                    sInt16 e_fid = paramP->convdef.fieldid /* +baseoffset */;
+                    sInt16 e_rep = repoffset;
+                    aItem.adjustFidAndIndex(e_fid,e_rep);
+                    // - get base field
+                    TItemField *e_basefldP = aItem.getField(e_fid);
+                    TItemField *e_fldP = NULL;
+                    if (e_basefldP)
+                      e_fldP=e_basefldP->getArrayField(e_rep,true); // get 
leaf field, if it exists
+                    if (!e_basefldP || e_fldP) {
+                      // base field of one of the main fields does not exist 
or leaf field is already in use
+                      // (unassigned is not good enough, otherwise we might 
end up adding information
+                      // to some other, previously parsed property using the 
same array field)
+                      // -> skip that repetition
+                      dostore=false;
+                      break;
+                    }
+                  }
+                  paramP=paramP->next;
+                }
                 // check if we can test more repetitions
                 if (!dostore) {
                   if (aRepArray[repid]+1<maxrep || maxrep==REP_ARRAY) {


> > I'm undecided now how to proceed. Simplify X-ABLabel property values so
> > that they can be stored as parameter? Use the more complex X-ABLabel
> > property and grouping? Bummer.
> 
> Really hard to say :-(
> 
> I'd say it depends on how important these ABLabels are (outside of
> Apple devices, that is. In iOS's Addressbook (AB), the Label IS
> important and actually represents the standard TYPE of a TEL, EMAIL or
> ADR as well, by using cryptic internal string constants like "_
> $!<Work>!$_" or "_$!<HomeFAX>!$_").

FWIW, Google CardDAV uses TYPE and only falls back to X-ABLabel for
custom labels and for X-ABRELATEDNAMES types.

For X-ABRELATEDNAMES, plain English nouns are used for well-known types
("Sister", "Parent") instead of using cryptic strings. Apple probably
uses cryptic strings to avoid conflicts with user-entered types. Google
doesn't seem to care, or perhaps intentionally maps a user-entered
"Sister" to the well-known type which gets translated in the Google web
UI.

I wonder how that works when syncing an Apple device with Google via
CardDAV. The Apple device must use the not-quite-internal vCard format
for CardDAV as seen from Google CardDAV, otherwise interoperability
would be limited. Indeed, it works.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.




_______________________________________________
os-libsynthesis mailing list
[email protected]
http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis

Reply via email to