410287 - calICalendar.getProperty returns always value type string

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[Sebastian Schwieger]

If setting a property boolean false via calICalendar.setProperty(), then calICalendar.getProperty() does not evaluate as expected.

For instance !cal.getProperty("someProperty") will not be true because it returns "0" as string.

I will attach a unit test as proof.

Comment 1

[Sebastian Schwieger]

Attachment: unit test for calICalendar.getProperty/setProperty

I needed to change calCalendarManager.js in order for the unit tests to work. Reason is, that there exists no profile for the unit tests and thus dbService.getProfileStorage("profile") always throws. 
The special case for 1.8 branch is not needed any more since it was indeed 1.8.0 branch specific.
The patch changes the "suppressAlarms" property using setProperty and checks the result using getProperty. It failes for storageCalendar since it returns always string values.

Comment 2

[Sebastian Schwieger]

Attachment: workaround

Proposed quick workaround.

Comment 3

[Daniel Boelzle [:dbo]]

Sebo, the WIP patch of bug 393395 incorporates a workaround for boolean properties unless we save properties into the prefs.

Comment 4

[Simon Paquet [:sipaq]]

This bug is not important enough to warrant wanted0.8 status. 

We would however take a fix for this bug into 0.8, if such a fix materializes.

Comment 5

[Sebastian Schwieger]

This bug blogs a wanted-0.8 bug. It is therefore important. A fix has materialized in bug 393395 (see comment 3).

Comment 6

[Sebastian Schwieger]

"blogs" should read "blocks", of course :-)

Comment 7

[Daniel Boelzle [:dbo]]

(In reply to comment #0)
> If setting a property boolean false via calICalendar.setProperty(), then
> calICalendar.getProperty() does not evaluate as expected.
> 
> For instance !cal.getProperty("someProperty") will not be true because it
> returns "0" as string.
This will be thoroughly fixed with bug 378754 when we shift calendar registration and prefs into the prefs. Unless that is done, we need to do the manual conversion in calProviderBase::get/setProperty. This has been done with bug 393395 for all known properties.

Comment 8

[Daniel Boelzle [:dbo]]

Comment on attachment 294918 [details] [diff] [review]
unit test for calICalendar.getProperty/setProperty

Besides that the patch contains some functions that IMO don't relate to this bug like compareItems, I think you should work on a registered calendar instead of manually setting the id.
If you wouldn't do that, then calProviderBase stores the props only transiently (IIRC bug 393395 changes that) which should be testable, too.

Comment 9

[Daniel Boelzle [:dbo]]

fixed with bug 403006