Bugzilla – Full Text Bug Listing
| Summary: | Drag and drop of overflown editor tabs is broken | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Markus Keller <markus.kell.r> | ||||
| Component: | UI | Assignee: | Patrik Suzzi <psuzzi> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | major | ||||||
| Priority: | P3 | CC: | daniel_megert, Lars.Vogel, psuzzi, sxenos | ||||
| Version: | 4.6 | Flags: | sxenos:
neon+
daniel_megert: pmc_approved+ |
||||
| Target Milestone: | 4.6.1 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows 7 | ||||||
| See Also: |
https://git.eclipse.org/r/76893 https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=00f6ec8ccbd5d17a1ddcc7015813487e643ce95f https://git.eclipse.org/r/79230 https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=617df2f89bc40e65101716d705cee18609f931f4 |
||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
Verified in Eclipse Platform
Version: Neon (4.6)
Build id: I20160606-1100
OS: Windows 10, v.10.0, x86_64 / win32
After analysis and test:
- DragAgent#dragStart(), #track() and #dragFinished() are properly called.
- The issue is in StackDropAgent#getDropIndex() that return wrong values as "itemRects", the list of editor tabs rectangles, is wrongly computed in case of editor tabs overflow.
e.g., see the sysout of mouse drag point vs editor tab rectangles
values with no tabs overflowing
CursorPos: Point {436, 123} | itemRects:[Rectangle {300, 108, 41, 24}, Rectangle {341, 108, 95, 24}, Rectangle {436, 108, 116, 24}, Rectangle {552, 108, 129, 24}, Rectangle {681, 108, 139, 24}, Rectangle {820, 108, 134, 24}]
values with tabs overflowing
cursorPos: Point {436, 118} | itemRects:[Rectangle {300, 108, 41, 24}, Rectangle {341, 108, 3862, 24}, Rectangle {4203, 108, -3763, 24}, Rectangle {440, 108, 123, 24}, Rectangle {563, 108, 133, 24}, Rectangle {696, 108, 204, 24}]
To solve the issue we need to fix how we compute the "itemRects", perhaps editing the createInsertRects() method.
Update: CTabFolder#getItems() returns a [] CTabItem. For some CTabs, those overflowing, the bounds are not correct. see: http://i.imgur.com/eUHpqiE.png New Gerrit change created: https://git.eclipse.org/r/76893 The main problem was the DnD and its feedback system, were not aware of hidden tabs, that we have when the tabs start to overflow in the editor. I solved using CTabItem#isShowing() to compute tabs visibility and creating a new list with visible tabs only. Then, I used the new list to compute insertion rectangles, at the beginning of Drag Operation; and to provide feedback during the mouse tracking . Please, see the attached change and share your feedback. I've left some comments on the code review. The general approach looks fine to me. Filtering based on item visibility is good. However, I had some suggestions regarding the details of the implementation. Created attachment 263509 [details]
anim gif: feedback and drop are ok
Thanks for the feedback.
I changed the code as suggested, and that lead me to a better solution.
With the latest change both feedback and drop are ok.
Gerrit change https://git.eclipse.org/r/76893 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=00f6ec8ccbd5d17a1ddcc7015813487e643ce95f Gerrit change https://git.eclipse.org/r/76893 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=00f6ec8ccbd5d17a1ddcc7015813487e643ce95f I think it will be good to backport this to Neon. Note: There are no publicly visible changes, except a comment added to a public method (In reply to Patrik Suzzi from comment #9) > I think it will be good to backport this to Neon. Sounds good. Please get a committer to review and set the +1 flag in this bug report (I think Stefan would be a good candidate), for bug fixes no PMC approval is necessary. (In reply to Lars Vogel from comment #10) > (In reply to Patrik Suzzi from comment #9) > > I think it will be good to backport this to Neon. > > Sounds good. Please get a committer to review and set the +1 flag in this > bug report (I think Stefan would be a good candidate), for bug fixes no PMC > approval is necessary. Right, but any fix needs either project lead or PMC approval. +1. (In reply to Dani Megert from comment #11) > +1. Please remember to set the flag in the correct bug flag field. (In reply to Lars Vogel from comment #12) > (In reply to Dani Megert from comment #11) > > +1. > > Please remember to set the flag in the correct bug flag field. There is no field for project_lead_approved ;-). (In reply to Dani Megert from comment #13) > (In reply to Lars Vogel from comment #12) > > (In reply to Dani Megert from comment #11) > > > +1. > > > > Please remember to set the flag in the correct bug flag field. > > There is no field for project_lead_approved ;-). So, acting as PMC now: +1 :-) Re: comment 10 I'm not sure how much good this will do without the rest of the drag & drop fixes that have gone into the 4.7 branch. However, I also think that this is a high quality low-risk fix that will have some end-user benefit... so I'll +1 a backport. Is this what you need from me as far as code review? Stefan, please set the +1 for the flag in the bug report. Done. New Gerrit change created: https://git.eclipse.org/r/79230 I cherry picked Change 76893 to Branch R4_6_maintenance to obtain Change 79230.
After testing Change 79230 on my local R4_6_maintenance, I encountered the the following issue:
!ENTRY org.eclipse.equinox.event 4 0 2016-08-17 22:47:12.813
!MESSAGE Exception while dispatching event org.osgi.service.event.Event [topic=org/eclipse/e4/ui/model/ui/UIElement/widget/SET] { (etc..) } to handler org.eclipse.e4.ui.internal.di.UIEventObjectSupplier$UIEventHandler@4962b41e
!STACK 0
java.lang.Error: Unresolved compilation problems:
The import java.util.stream cannot be resolved
The import java.util.stream cannot be resolved
Stream cannot be resolved
Lambda expressions are allowed only at source level 1.8 or above
Collectors cannot be resolved
at org.eclipse.e4.ui.workbench.addons.dndaddon.StackDropAgent.<init>(StackDropAgent.java:17)
>> Full paste here: http://pastebin.com/TMYiFCRh
>> Image here: http://imgur.com/eD8SGZ7.png
That is because the plug-in org.eclipse.e4.ui.workbench.addons.swt supports Java 1.7 in Neon. To solve, I should slightly change the code removing Java8 Lambdas.
Hence, my question is:
Should I rewrite the code in Change 79230 (Neon maintenance) to be Java 1.7 Compliant, despite it will be different W.R.T. the code in Change 76893 (Oxygen)?
In the second change provided, https://git.eclipse.org/r/#/c/79230/2, I just edited the code to be Java 1.7 Compliant, see: https://goo.gl/HJgmwK (In reply to Patrik Suzzi from comment #19) > Hence, my question is: > Should I rewrite the code in Change 79230 (Neon maintenance) to be Java 1.7 > Compliant, despite it will be different W.R.T. the code in Change 76893 > (Oxygen)? Yes. Change is good for RC3. Please wait until RC2 is declared. Gerrit change https://git.eclipse.org/r/79230 was merged to [R4_6_maintenance]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=617df2f89bc40e65101716d705cee18609f931f4 Verified in M20160831-0700 (4.6.1 RC3). Tested original scenario and other setups in new and old workspaces and with different themes. |
4.6, 4.5.2, and 4.5. Was better in 4.4.2. Drag and drop of overflown editor tabs is broken. Steps: - new workspace - paste to Package Explorer: public class C { public static void main(String[] args) { } } - Ctrl+Click type String and then a few other types until editor tabs start to overflow - use Ctrl+F6 (press F6 multiple times) to activate C.java - try to drag C.java to an different position => no drop targets appear, and the tab stays at the front