466439 - calIRelation/calIAttachment need proper cloning facility

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[Daniel Boelzle [:dbo]]

Those objects store a reference to their base item. In case of cloning a calendar item, currently only the set of relations/attachments is cloned, leaving a reference to the item that has been cloned.

Comment 1

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Daniel, are willing to fix this one? If so, then please mark blocking+, otherwise blocking- and wanted+.

Comment 2

[Martin Schröder [:mschroeder]]

Attachment: Patch v1

This is a first try (also my first time dealing with the backend)... Daniel, it would be great to get some input from you if this patch heads in the right direction.

Comment 3

[Daniel Boelzle [:dbo]]

Comment on attachment 382115 [details] [diff] [review]
Patch v1

calIAttachment::item, calIRelation::item still seem to be wrong (refer to the old item). However, ad hoc I don't remember why we need to refer to the item at all? Philipp, you do?

Comment 4

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

If possible we should get rid of the item attribute to avoid circular references. Since calIRelation is not used yet, we might get away with removing it until we find out we need it.

I don't see why calIAttachment would need an item.

Comment 5

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

I don't think this should block 1.0. Attachments and Relations are not really highly used features and they are not broken per se.

Comment 6

[Stefan Sitter [:ssitter]]

Sounds like something that would benefit from a xpcshell testcase.

Comment 7

[Martin Schröder [:mschroeder]]

Attachment: [checked in] Patch v1

This patch still has the cloning facility but the references to an item have been removed.

Comment 8

[Daniel Boelzle [:dbo]]

Comment on attachment 384294 [details] [diff] [review]
[checked in] Patch v1

looks good; r=dbo

Comment 9

[Martin Schröder [:mschroeder]]

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/bab2dcc9b4de>

I'll adapt the existing unit tests, and add additional ones next week.

Comment 10

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".