Closed Bug 357112 Opened 13 years ago Closed 10 years ago

Drag and drop of multiday-event doesn't drop on days the shadow suggests

Categories

(Calendar :: Calendar Views, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: omarb.public, Assigned: bv1578)

References

Details

(Keywords: regression, Whiteboard: [not needed beta][no l10n impact])

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.8.1) Gecko/20061010 Firefox/2.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.8.1) Gecko/20061010 Firefox/2.0

Drag and drop of tasks/events doesn't work correctly.

Reproducible: Always

Steps to Reproduce:
1.Create an event/task that lasts for example a week (say from 16/10/2006 to 22/10/2006)
2.click on 2nd, 3rd,...,7th day of event and drag it to other week
3.Before releasing the mouse button notice what you see
4.Release the button

Actual Results:  
The'shadow' doesn't cover the result of the operation

Expected Results:  
When you click for example on 2nd day of task/event and drag it to next week you expect it will be a second day of the task/event not first.

Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.9a1) Gecko/20061006 Sunbird/0.3
Confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061027 Calendar/0.4a1.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Drag and drop function doesn't work correctly. → Drag and drop of multiday-event does't drop on days the shadow suggests
Version: unspecified → Trunk
Shouldn't it be fixed before releasing calendar 0.5?
Flags: blocking-calendar0.5?
For the life of me, I still can't understand the problem or the fix in bug 354633.  However, assuming that fix really is needed to fix some bug, this patch keeps it around and still includes the offset necessary for the calculations.
Assignee: nobody → jminta
Status: NEW → ASSIGNED
Attachment #248039 - Flags: second-review?(dmose)
Attachment #248039 - Flags: first-review?(ctalbert.moz)
Comment on attachment 248039 [details] [diff] [review]
[checked in] include offset in calculation

Reviewed and tested on windows. This solves the problem. The reason it works is it takes into account the offset of what the user clicked on in relation to the start/end time of the event. Looks good. ctalbert=r+
Attachment #248039 - Flags: first-review?(ctalbert.moz) → first-review+
Attachment #248039 - Flags: second-review?(dmose)
jminta:
Does this just need checkin?
Whiteboard: [needs checkin?]
Flags: blocking-calendar0.5? → blocking-calendar0.5+
jminta said in IRC that he thought it just needed checkin.

Patch checked in on MOZILLA_1_8_BRANCH and trunk.

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [needs checkin?]
Verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.3pre) Gecko/20070321 Calendar/0.5pre
Status: RESOLVED → VERIFIED
Problem is again present with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.7pre) Gecko/20070831 Calendar/0.7pre and lightning 2007083103----> reopening
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Whiteboard: regression
Keywords: regression
Whiteboard: regression
Assignee: jminta → nobody
Status: REOPENED → NEW
(In reply to comment #8)
I really wish people would file new bugs for new regressions and just refer to the old bugs instead of reopening the old bugs that were fixed and verified for previous releases.
(In reply to comment #9)
> I really wish people would file new bugs for new regressions and just refer to
> the old bugs instead of reopening the old bugs that were fixed and verified for
> previous releases.
> 

That is why I asked on chat what I should do- I was told to reopen the bug. If I did wrong, sorry for that. Feel free to close it and open a new one for me, please.
Flags: wanted-calendar0.8?
Setting wanted0.8- as the main Calendar developers will not devote any time to
this in the 0.8 timeframe. Patches are, of course, always welcome.
Flags: wanted-calendar0.8?
Flags: wanted-calendar0.8-
Flags: blocking-calendar0.5+
Attachment #248039 - Attachment description: include offset in calculation → [checked in] include offset in calculation
Depends on: 416206
Flags: wanted-calendar0.8-
Summary: Drag and drop of multiday-event does't drop on days the shadow suggests → Drag and drop of multiday-event doesn't drop on days the shadow suggests
This patch adds the code to figure out the offset of the dragged element for spanning days events and tasks in month-view and all-day events in week-view.

Instead of 'parentBox', the property 'parentNode' could be used, in this case it doesn't need to add the property inside binding calendar-header-container. Please tell me what's better.
Attachment #397520 - Flags: review?(philipp)
Attachment #397520 - Flags: review?(philipp) → review+
Comment on attachment 397520 [details] [diff] [review]
Fix for Calendar 1.0 (again include offset in calculation)

>+         item = session.sourceNode.sourceObject;
Missing let.


Otherwise the code looks fine. Not sure how to best handle the parentNode/Box stuff. This makes the box depend on its parent. Maybe we can just pass the date instead?
Assignee: nobody → bv1578
Status: NEW → ASSIGNED
Flags: in-litmus?
OS: Windows XP → All
Hardware: x86 → All
(In reply to comment #13)
> (From update of attachment 397520 [details] [diff] [review])
> >+         item = session.sourceNode.sourceObject;
> Missing let.
>

original code defines the 'item' variable twice (see lines 450 and 469 in calendar-widgets.xml file), so I deleted the second 'let'.
http://mxr.mozilla.org/comm-central/source/calendar/base/content/widgets/calendar-widgets.xml#450ntral/source/calendar/base/content/widgets/calendar-widgets.xml#450

> 
> Otherwise the code looks fine. Not sure how to best handle the parentNode/Box
> stuff. This makes the box depend on its parent. Maybe we can just pass the 
> date instead?

I have problems in doing this in different ways, i.e. I haven't found another easy way to get the date of the starting point of the drag session other than the date of the box of the dragged element (session.sourceNode).
Is there a simple way instead?

Actually the property 'parentBox' should belong to event boxes instead of their box-container like 'calendar-header-container'.
I had thought to move the parentBox property inside 'calendar-editable-item' binding to make that property available for events boxes in month-view AND for all-day ones in week-view (because now event boxes for all-day events in the week-view are added with createXulElement() function and don't have a binding like 'calendar-month-day-box-item' where to add a property 'parentBox'). But I don't know if this would be a mistake i.e. 'calendar-editable-item' binding is a bad place for a 'parentBox' property (otherwise I've already tried a working patch in this way).
Comment on attachment 397520 [details] [diff] [review]
Fix for Calendar 1.0 (again include offset in calculation)

Decathlon, what is the state of this patch? It has review+, but it seems there are some rough edges Who needs to act next, and how can we help?
I'm waiting a review for the patch attached to Bug 476227 because that one would fix this one too (this issue is only a part of that other one).
I had to write a comment here, sorry.

Actually the problem related to the 'parentBox' mentioned in comment #13 is the same in the other patch because I've basically used the same code for the same patch's part.
I've mentioned this problem in bug 476227 comment #2. Maybe after the review there, I will need to ask the same question that I did here in * comment #14 * (I had written a mail to Philipp too, but he is too busy ;-). With a negative answer to this question I will try to do in a different way, but if I remember correctly (over three months have past) I will need some suggestion ;-).
Let's wait for Philipp's review on bug 476227.
This bug has been fixed with checkin for bug 476227.
Could someone confirm?
Flags: blocking-calendar1.0+
Whiteboard: [not needed beta][no l10n impact]
Confirmed
Status: ASSIGNED → RESOLVED
Closed: 13 years ago10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0b2
Flags: in-litmus?
You need to log in before you can comment on or make changes to this bug.