Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 320338 - SplitDropAgent should not use client area for preview rectangle
Summary: SplitDropAgent should not use client area for preview rectangle
Status: RESOLVED FIXED
Alias: None
Product: e4
Classification: Eclipse Project
Component: UI (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 1.0 RC3   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-19 19:14 EDT by Stefan Mücke CLA
Modified: 2010-07-27 14:56 EDT (History)
3 users (show)

See Also:
emoffatt: review+
emoffatt: review?


Attachments
screenshot (47.54 KB, image/png)
2010-07-19 19:14 EDT, Stefan Mücke CLA
no flags Details
patch (1.62 KB, patch)
2010-07-21 01:43 EDT, Stefan Mücke CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Mücke CLA 2010-07-19 19:14:54 EDT
Created attachment 174688 [details]
screenshot

The preview rectangle for dragging parts is inconsistent and IHMO wrong for view stacks (because the dragged view will not be placed inside of the drop target). See screenshot.

It seems there is code in SplitDropAgent.getRectangle(...) that was meant to fetch the CTabFolder for computing the rectangle, but this code fails because there is another Composite between the drop part's widget and the CTabFolder.

A fix would be to add another "if" to check the parent's parent, or to always check for the parent's parent (if the intermediate Composite always exists).

  if (ctrl.getParent() instanceof CTabFolder)
      ctrl = ctrl.getParent();
  else if (ctrl.getParent().getParent() instanceof CTabFolder)
      ctrl = ctrl.getParent().getParent();
Comment 1 Stefan Mücke CLA 2010-07-19 19:40:37 EDT
The intermediate Composite is created by the TrimmedPartLayout.
Comment 2 Stefan Mücke CLA 2010-07-21 01:43:34 EDT
Created attachment 174815 [details]
patch

Boris, any chance to get this into RC3?

The code is safe; I added a null check to make sure it can never result in an NPE.
Comment 3 Remy Suen CLA 2010-07-22 11:11:47 EDT
Looks safe to me and I do agree that this discrepancy is odd and should be corrected.
Comment 4 Eric Moffatt CLA 2010-07-22 11:28:41 EDT
Committed in >20100722. Applied the patch.
Comment 5 Eric Moffatt CLA 2010-07-22 11:29:09 EDT
Thanks Stefan!
Comment 6 Stefan Mücke CLA 2010-07-22 11:30:44 EDT
Thanks for applying the patch. Now I can sleep again ;-)
Comment 7 Stefan Mücke CLA 2010-07-22 11:42:00 EDT
Oh no, I forgot that bug 320464 is still open... ;-)
Comment 8 Eric Moffatt CLA 2010-07-27 14:56:56 EDT
Verified on XP in I20100726-2152.