341154 - calIIcalProperty icalString attribute

Status: VERIFIED FIXED

(Calendar :: Internal Components, enhancement)

Description

[Daniel Boelzle [:dbo]]

Attribute to get the whole ical property object as an escaped string.

Comment 1

[Daniel Boelzle [:dbo]]

Attachment: icalString attribute + implementation

Comment 2

[Daniel Boelzle [:dbo]]

Attachment: icalString attribute + implementation

revised patch to apply again

Comment 3

[Daniel Boelzle [:dbo]]

Attachment: revised calIIcalPropery icalString

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Can you post the WCAP provider code somewhere so it's possible to see what situations this is used in?

Comment 5

[Daniel Boelzle [:dbo]]

(In reply to comment #4)
> Can you post the WCAP provider code somewhere so it's possible to see what
> situations this is used in?

I had a look at the current code and see that this new attribute is currently (only) used for logging. I needed it some months ago and introduced it; IMO it is likely to be used again (because the sun server takes arguments in ical mimic) and a sensible enhancement.
Dan, I would like check in the code, but still need to know whether it is ok to add provider specific IDL to base/public, i.e.
- error codes to calIErrors
- calIFreeBusyEntry
- calIFreeBusyListener
- calIWCapCalendar
before checking in.

Comment 6

[Matthew (lilmatt) Willis]

(In reply to comment #5)
> Dan, I would like check in the code, but still need to know whether it is ok
> to add provider specific IDL to base/public

I'll try and save Dan some keystrokes here.
I believe that rather than checking in, what he was suggesting was to create a diff and post it somewhere for him to be able to look at.

Comment 7

[Daniel Boelzle [:dbo]]

(In reply to comment #6)
> (In reply to comment #5)
> > Dan, I would like check in the code, but still need to know whether it is ok
> > to add provider specific IDL to base/public
> 
> I'll try and save Dan some keystrokes here.
> I believe that rather than checking in, what he was suggesting was to create a
> diff and post it somewhere for him to be able to look at.
> 

Done. Please follow up in issue 340949.

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 225579 [details] [diff] [review]
revised calIIcalPropery icalString

> NS_IMETHODIMP
>+calIcalProperty::GetIcalString(nsACString &str)
>+{
>+    char const* icalstr = icalproperty_as_ical_string(mProperty);
>+    if (icalstr == 0) {
>+#ifdef DEBUG
>+        fprintf(stderr, "Error getting ical string: %d (%s)\n",
>+                icalerrno, icalerror_strerror(icalerrno));
>+#endif
>+        return NS_ERROR_FAILURE;

If you would, use

 return calIErrors::ICS_ERROR_BASE + icalerrno;

and also add an @exception clause to the IDL documentation making it clear that any libical error will be thrown as an exception.

r=dmose with that fixed

Thanks for the patch.

Comment 9

[Daniel Boelzle [:dbo]]

landed.