Closed
Bug 954941
Opened 10 years ago
Closed 10 years ago
Drop target does not reappear after detaching all but one merged buddies
Categories
(Instantbird Graveyard :: Contacts window, defect)
Instantbird Graveyard
Contacts window
Tracking
(Not tracked)
RESOLVED
FIXED
1.2
People
(Reporter: aleth, Assigned: aleth)
Details
Attachments
(3 files, 2 obsolete files)
1.23 KB,
patch
|
clokep
:
review-
|
Details | Diff | Splinter Review |
1.62 KB,
patch
|
clokep
:
review+
|
Details | Diff | Splinter Review |
1.76 KB,
patch
|
clokep
:
review-
|
Details | Diff | Splinter Review |
*** Original post on bio 1509 at 2012-06-12 16:04:00 UTC *** Timestamp: 06/12/2012 05:58:26 PM Error: this.childNodes[1] is undefined Source File: chrome://instantbird/content/contact.xml Line: 170 STR - Merge a buddy by drag and drop onto the drop target of another buddy - Note drop target disappears - Detach the added buddy - Drop target does not reappear - Drag the buddy back onto the contact to merge it: error
Assignee | ||
Comment 1•10 years ago
|
||
*** Original post on bio 1509 as attmnt 1609 at 2012-06-14 22:35:00 UTC *** Checks the drop target is there before trying to remove it.
Attachment #8353366 -
Flags: review?(clokep)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•10 years ago
|
||
*** Original post on bio 1509 as attmnt 1610 at 2012-06-14 22:37:00 UTC *** Alternatively, put back the drop target immediately when detaching the last merged buddy. This seems better in some ways, but it "feels odd" somehow when actually using it. What do you think?
Attachment #8353367 -
Flags: review?(clokep)
Assignee | ||
Comment 3•10 years ago
|
||
*** Original post on bio 1509 as attmnt 1611 at 2012-06-14 22:53:00 UTC *** Same as the alternative patch, but with a setTimeout so the detach animation finishes before the drop target is added back. Gets rid of the odd feeling of the alternative patch imho.
Attachment #8353368 -
Flags: review?(clokep)
Assignee | ||
Comment 4•10 years ago
|
||
*** Original post on bio 1509 as attmnt 1612 at 2012-06-14 23:10:00 UTC *** Forgot to trigger the insert animation for the drop target.
Attachment #8353369 -
Flags: review?(clokep)
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8353368 [details] [diff] [review] Alternative patch with delay *** Original change on bio 1509 attmnt 1611 at 2012-06-14 23:10:44 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353368 -
Attachment is obsolete: true
Attachment #8353368 -
Flags: review?(clokep)
Assignee | ||
Comment 6•10 years ago
|
||
*** Original post on bio 1509 at 2012-06-14 23:27:35 UTC *** There is still a bug here. If, after detaching the buddy, I drop another buddy onto the remaining buddy inside the contact, I get the following error Timestamp: 06/15/2012 01:00:47 AM Error: observer.observe is not a function Source File: components/imContacts.js Line: 1091 So it seems I am missing something...
Assignee | ||
Comment 7•10 years ago
|
||
*** Original post on bio 1509 at 2012-06-14 23:34:34 UTC *** (In reply to comment #5) > There is still a bug here. To be more specific, this only applies to the alternative patches.
Assignee | ||
Comment 8•10 years ago
|
||
*** Original post on bio 1509 at 2012-06-15 00:14:31 UTC *** (In reply to comment #6) > (In reply to comment #5) > > There is still a bug here. > To be more specific, this only applies to the alternative patches. The problem is that buddyElt.build(aSubject, this); adds an observer. And the code is not set up to have two buddies to transition to. I can't see how to add the insert transition animation "by hand" without causing an error somewhere down the line. Leaving the problematic line out, I can still produce an error Timestamp: 06/15/2012 02:08:47 AM Error: this._observers is undefined Source File: file://components/imContacts.js Line: 808 unless I remove the setTimeout call. The error occurs when dropping a buddy onto the remaining buddy after detaching the merged buddy. So it's only the "alternative Patch", without buddyElt.build(aSubject, this), that appears to be as bug-free as the original Patch.
Assignee | ||
Comment 9•10 years ago
|
||
Comment on attachment 8353369 [details] [diff] [review] Alternative patch with delay *** Original change on bio 1509 attmnt 1612 at 2012-06-15 00:16:19 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353369 -
Flags: review?(clokep) → feedback?(clokep)
Assignee | ||
Comment 10•10 years ago
|
||
*** Original post on bio 1509 as attmnt 1613 at 2012-06-15 00:33:00 UTC *** Found a way to add the delay without (as far as I can tell) adding any bugs. To repeat, it would be nice if one could use the transition timer mechanism for this, but I don't see how to make this work for a drop target buddy and for transitioning to two buddies (detached buddy and drop target) rather than just one (the detached buddy).
Attachment #8353370 -
Flags: review?(clokep)
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8353369 [details] [diff] [review] Alternative patch with delay *** Original change on bio 1509 attmnt 1612 at 2012-06-15 00:33:54 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353369 -
Attachment is obsolete: true
Attachment #8353369 -
Flags: feedback?(clokep)
Comment 12•10 years ago
|
||
Comment on attachment 8353367 [details] [diff] [review] Alternative patch *** Original change on bio 1509 attmnt 1610 at 2012-06-15 21:39:01 UTC *** I definitely like this one the best FWIW.
Attachment #8353367 -
Flags: review?(clokep) → review+
Comment 13•10 years ago
|
||
Comment on attachment 8353366 [details] [diff] [review] Patch *** Original change on bio 1509 attmnt 1609 at 2012-06-15 21:39:20 UTC *** This is pretty clearly broken if the drop target doesn't come back.
Attachment #8353366 -
Flags: review?(clokep) → review-
Comment 14•10 years ago
|
||
Comment on attachment 8353370 [details] [diff] [review] Alternative patch with delay *** Original change on bio 1509 attmnt 1613 at 2012-06-15 21:39:44 UTC *** I think the delay makes this one look broken...
Attachment #8353370 -
Flags: review?(clokep) → review-
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Comment 15•10 years ago
|
||
*** Original post on bio 1509 at 2012-06-21 00:29:18 UTC *** Committed as http://hg.instantbird.org/instantbird/rev/8d161d706eb2
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.2
You need to log in
before you can comment on or make changes to this bug.
Description
•