Closed Bug 393969 Opened 13 years ago Closed 13 years ago
After deleting an event, some events overlapps
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
Confirmed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:184.108.40.206pre) 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.
Reduce the width of the application window and you get the same overlapping events in the day view.
I set this issue to 'blocking 0.7?' because this bug is a regression and looks ugly.
Granting blocking 0.7 as the views should definitely work without any flaws.
Flags: blocking-calendar0.7? → blocking-calendar0.7+
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...
This patch contains what I tried to explain in my previous comment...
Attachment #279094 - Flags: review?(daniel.boelzle)
Comment on attachment 279094 [details] [diff] [review] patch v1 r=dbo
Attachment #279094 - Flags: review?(daniel.boelzle) → review+
patch checked in on trunk and MOZILLA_1_8_BRANCH -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Checked issue in latest nightly (20070905) -> set task to verified
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.