393969 - After deleting an event, some events overlapps

Status: VERIFIED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[Andreas Treumann]

STEPS TO REPRODUCE:
===================

- create a new local calendar
- import the attached test.ics file
- delete the 'delete me!' event

RESULT:
=======

- 'event 2' overlapps 'event 1'

EXPECTED RESULT:
================

- 'event 2' should not overlapps 'event 1'

REPRODUCIBLE:
=============

- always

Comment 1

[Andreas Treumann]

Attachment: ics-file to reproduce this bug

Comment 2

[Stefan Sitter [:ssitter]]

Confirmed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.7pre) Gecko/2007082801 Calendar/0.7pre in WEEK view only.

In DAY view the events do not overlap. But the event 'New Event' (2007-08-30 10:00 UTC - 2007-08-30 11:00 UTC) takes up less space than available.

Comment 3

[Andreas Treumann]

Reduce the width of the application window and you get the same overlapping events in the day view. 

Comment 4

[Andreas Treumann]

I set this issue to 'blocking 0.7?' because this bug is a regression and looks ugly. 

Comment 5

[Michael Büttner (no reviews TFN)]

Granting blocking 0.7 as the views should definitely work without any flaws.

Comment 6

[Michael Büttner (no reviews TFN)]

The new layout code is a fair bit more complex than it was the case with the previous incarnation, but I need to dig into the details in order to explain what's going wrong.

After an initial step which is responsible for finding some additional information for each event, we end up with an array that tells us for each event a) how many columns it spans and b) on which column it should start. Something like in the following example:

----
|a |------------
|  ||b         |
|  |------------
|  |----
|  ||c |
|  ||  |--------
|  ||  ||d     |
|  ||  |--------
|  ||  |--------
|  ||  ||e ||f |
----------------

a: start - 0 span - 1
b: start - 1 span - 3
c: start - 1 span - 1
d: start - 2 span - 2
e: start - 2 span - 1
f: start - 3 span - 1

From this representation we're going to build a 3d array where the first dimension defines a set of layers (z coordinate), the second dimension defines the individual columns (x coordinate) and the third dimension defines the time each individual item consumes (y coordinate). The first dimension is probably the most important one as it represents the set of columns of events that all should have equal width. This actually is going to end up in different xul-stacks.

We're building this 3d array from the first representation, where one piece of the puzzle is to find the layer each individual item will end up on while trying to keep the number of layers to the absolute minimum. Basically, we need to distribute the items on different layers depending on the number of columns each item spans. if each item just spans a single column the bunch of items will cover a single layer. if the chunk of items contains items that span more than a single column, we'll end up with more layers. the algorithm that finds out on which layer each item needs to go uses a sparse matrix based on the number of spans used and the starting column of each individual item.

  0 1 2 3 4
0
1       *
2     *
3
4

In this figure the rows mean the starting column (0 to 4 in this case) and the columns mean the number of spans (also 0 to 4 in this case). For each combination of start/span we create a new layer (where I placed the '*' in the figure as an example).

Unfortunately, the code that currently calculates the offset simply says

index = span * (start+1)

In the above example both combinations end up with index 6:

6 = 3*(1+1)
6 = 2*(2*1)

Which means both items end up using the same layer even if they occupy a different number of columns. That's definitely wrong. The simple cure to the problem is to not multiply span and start but to state:

index = 'number of columns' * span + start;

This ensure that different combinations of start/spans end up on different layers...

Comment 7

[Michael Büttner (no reviews TFN)]

Attachment: patch v1

This patch contains what I tried to explain in my previous comment...

Comment 8

[Daniel Boelzle [:dbo]]

Comment on attachment 279094 [details] [diff] [review]
patch v1

r=dbo

Comment 9

[Michael Büttner (no reviews TFN)]

patch checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED

Comment 10

[Andreas Treumann]

Checked issue in latest nightly (20070905) -> set task to verified