Community
Participate
Working Groups
Created attachment 214199 [details] Screenshot View tab folder 'leaks' (i.e. is not fully closed) in Classic theme. Screenshot shows 3.8 in the background for comparison.
I see the same bug when I use SWT from the master branch with Eclipse SDK 3.8RC4, (applying this workaround from bug 378000 comment 4: > Commenting out the resize handler in PaneFolder will allow you to come up.) Please move to SWT if the bug is there.
Here's a snippet that shows this in pure SWT (thanks Carolyn!): import org.eclipse.swt.*; import org.eclipse.swt.custom.*; import org.eclipse.swt.graphics.*; import org.eclipse.swt.layout.*; import org.eclipse.swt.widgets.*; public class CTabFolderManyTabsTest { public static void main(String[] args) { Display display = new Display(); Shell shell = new Shell(display); shell.setLayout(new GridLayout()); CTabFolder tabFolder = new CTabFolder(shell, SWT.BORDER); tabFolder.setSimple(false); tabFolder.setMaximizeVisible(true); tabFolder.setMinimizeVisible(true); tabFolder.setSelectionBackground(new Color[] { display.getSystemColor(SWT.COLOR_WHITE), display.getSystemColor(SWT.COLOR_WHITE), display.getSystemColor(SWT.COLOR_BLUE)}, new int[] {50, 100}, true); for (int i = 0; i < 12; i++) { CTabItem item = new CTabItem(tabFolder, SWT.NULL); item.setText("Tab " + i); StyledText itemText = new StyledText(tabFolder, SWT.MULTI); itemText.setText("\n\tText control for tab " + i + "\n\n\n"); item.setControl(itemText); } shell.pack(); shell.open(); while (!shell.isDisposed()) { if (!display.readAndDispatch()) display.sleep(); } } }
The trim needs 2 more pixels on the right, so that the max/min toolbar doesn't clip the parent's border. Here's a quick one-line change that works for this snippet, but I don't know if it works for all cases (mirrored, tabs on bottom, no border, different renderers, etc). BG, can you have a look? In CTabFolderRenderer.computeTrim() change line 2 from: int borderRight = borderLeft; to: int borderRight = parent.borderVisible ? 3 : 0;
(In reply to comment #3) > int borderRight = parent.borderVisible ? 3 : 0; Hmm, that removes the 'leak', but it also moves the Maximize button even more to the left. The visually most pleasing solution is to have the same amount of pixels from the Maximize box bounds to all border lines of the CTabFolder (right, top, bottom). You probably have to draw the corner as background of the ToolBar, since SWT doesn't support glass panes (bug 114749).
The toolbar is exactly the same size as the two buttons - i.e. it does not add any extra space, so you would actually have to draw on the tool button, not just on the toolbar. I talked to SSQ and BG, and they think it would be difficult to get this right on all platforms because the tool button background changes with hover, pressed, etc. Is the "extra 2 pixels" solution acceptable? I am not sure how many people are using the "Classic" theme - do you guys use it all the time? There might be another solution (somewhere in computeTrim) but I am out of time to look at this further at the moment.
Created attachment 223627 [details] Fix (In reply to comment #5) > Is the "extra 2 pixels" solution acceptable? Nope. It closes the 'leak', but it's not correct to set the whole right border to 3px (i.e. not only in the header but also in the whole content area). This e.g. makes the right border of the Package Explorer wider than the left border (easier to see if the view is not active). The real issue is that the rounded border is drawn into the PART_HEADER area. The solution should be applied where the problem happens: CTabFolderRenderer#computeTrim(..) should be implemented for PART_HEADER, and CTabFolder#computeControlBounds(..) should respect that trim. The attached patch implements this and fixes the 'leak'. However, when I was almost done, I realized that the 2px are quite strange. The rounded corner occupies 5px. I guess the remaining 3px are CTabFolder's magic SPACING. The patch also avoids wasting even more space on the right by making the Maximize button narrower. For that, I had to split the minMaxTb into two (bug 287650). > I am not sure how many people > are using the "Classic" theme - do you guys use it all the time? Yes, to test API compatibility and due to the plethora of UX flaws in E4.
Created attachment 223628 [details] Fix Same as first patch, but makes the connection to CTabFolder.SPACING explicit.
I can't apply the patch cleanly. Can you refresh it with the latest, please?
Created attachment 223657 [details] Fix 3 Rebased on top of bug 394016.
OK, I had a look at the patch in comment 9, and it looks ok for Classic theme. But if you switch to Windows 7 theme, the Maximize button becomes very narrow. This is because the "header trim" is a different size for different renderers. FYI, I am just pasting the following code here so that it is not lost (this is the "right" way to do the simple fix from comment 3, but it still has the visual problem of moving the tools 2px to the left): In CTabFolderRenderer.computeTrim(), change the PART_BORDER case to: case PART_BORDER: x = x - borderLeft - 2; width = width + borderLeft + borderRight + 4; y = y - borderTop; height = height + borderTop + borderBottom; break; I talked to SSQ and BG, and if possible it would be nice to stick to one toolbar so that we use fewer controls. Maybe both tool images can be made smaller?
Comment on attachment 223657 [details] Fix 3 Thanks for checking comment 9 against the Windows 7 theme. I agree that cannot be released. I filed bug 394803 to fix the black magic around the meaning of the PART_* constants. This will probably reveal more problems in CTabFolderRenderer and E4's CTabRendering where they just work by chance right now. (In reply to comment #10) This looks OK for now. It makes e.g. the Package Explorer 10px wider than in 3.8 if you want to fit all controls in one row, but that's still better than the gap in the border.
So shall we go ahead with the "x -= 2; width += 4;" fix for now, and mark this bug fixed?
We are considering making the 'Windows 7 Classic' theme the default again, but for that we should first get this bug fixed. Lakshmi, please take a look at this during M4.
Created attachment 237133 [details] minimal fix This is comment 10, but without wasting 2 pixels on the left of the toolbar. I'd go with this one.
The +2 is actually only necessary for the CTabFolder.simple case, where CTabFolderRenderer.TOP_RIGHT_CORNER needs more space. SIMPLE_TOP_RIGHT_CORNER is fine. Fixed with http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=ef0e9ff12c5d3d428d50a1e38e38f3baf1a24c9d
Verified in N20131106-2000.
