Community
Participate
Working Groups
Created attachment 265374 [details] Screenshot Several bugs have been introduced in CTabFolder in the 4.7 versions. See attached screenshot. - the initial layout and/or preferred size is incorrect; note that the item's tab content is not visible after the initial pack(); when pack() is invoked using the button, the shell is sized correctly - selection background isn't rendered when the folder doesn't have focus - the icon is drawn on top of the item's border The bugs can be reproduced with the following snippet: public static void main(String[] args) { Display display = new Display(); Shell shell = new Shell(display); shell.setLayout(new GridLayout(1, false)); shell.setText("SWT Version " + SWT.getVersion()); Text text2 = new Text(shell, SWT.SINGLE | SWT.LEAD | SWT.BORDER); text2.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, false)); text2.setText("This control takes the initial focus."); Button button = new Button(shell, SWT.PUSH); button.setText("Shell.pack()"); button.addListener(SWT.Selection, e -> shell.pack()); Color red = new Color(display, 255, 0, 0); CTabFolder folder = new CTabFolder(shell, SWT.BORDER); folder.setLayoutData(new GridData(SWT.FILL, SWT.BEGINNING, true, false)); folder.setSelectionBackground(red); for (int i = 0; i < 3; i++) { CTabItem item = new CTabItem(folder, SWT.CLOSE); item.setText("Item " + i); Text text = new Text(folder, SWT.MULTI); text.setText("Content for Item " + i); item.setImage(display.getSystemImage(SWT.ICON_INFORMATION)); item.setControl(text); } folder.setSelection(0); shell.pack(); shell.open(); while (!shell.isDisposed()) { if (!display.readAndDispatch()) display.sleep(); } display.dispose(); }
Created attachment 265384 [details] regressions on gtk 3.14 Seem to be not Windows only, see screenshots from RHEL 7.2 (gtk 3.14).
I could reproduce this on Win7 and Ubuntu16.04 With some initial investigation and looks like the refactoring work done in CTabFolder via bug 474444 has broken CTabFolder initial focus.
Possible reason for broken CTabFolder's initial focus: It seems the CTabFolder's 'highlight' flag is not turned 'on' when CTabFolder.setSelection(int index) method is called. By making above change I am able to resolve this focus rendering problem in CTabFolder, will share a Gerrit patch shortly.
Thanks for this good quality report! Computing the size indeed seems to be a "fixable" bug. The icon being drawn on top of the border too (although it doesn't look to bad in this example) For the selection color, ideally the CTabFolder would need to have 2 selection background: * The regular selection background * The "noFocus" selection background. Having both is important to identify which tab has the focus (which is important as reported on bug 474444). Maybe we can find a better way to decide of a noFocus color: if selection background was explecitly set, reuse it (and reintroducee bug 474444 - but at least it's "on demand"), or maybe try to generate a proper "noFocus" background that would be a mix of the selection background and the non-selected one. Then, still about this selection background: a workaround could be to add listener on the CTabItem and dymanically change background color according to selection... WDYT?
(In reply to Niraj Modi from comment #3) > By making above change I am able to resolve this focus rendering problem in > CTabFolder, will share a Gerrit patch shortly. I believe that will also re-introduce the usability issue of bug 474444.
Could the issue about size be more related to bug 502111 and http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=8a456e1b2a83efab27513cfa8ed8f72ab32fa6e0 ? Bug 474444 should only be about colors and, unless mistake, doesn't change anything is size computation.
(In reply to Mickael Istria from comment #5) > (In reply to Niraj Modi from comment #3) > > By making above change I am able to resolve this focus rendering problem in > > CTabFolder, will share a Gerrit patch shortly. > > I believe that will also re-introduce the usability issue of bug 474444. Yes with this approach, it does re-introduce the usability issue of bug 474444. (In reply to Mickael Istria from comment #6) > Could the issue about size be more related to bug 502111 and > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/ > ?id=8a456e1b2a83efab27513cfa8ed8f72ab32fa6e0 ? Sizing problem seems to exist in 4.7M2 as well(bug 502111 went in 4.7M3)
(In reply to Niraj Modi from comment #7) > Sizing problem seems to exist in 4.7M2 as well(bug 502111 went in 4.7M3) Commit 8a456e1b2a83efab27513cfa8ed8f72ab32fa6e0 was merged on 2016-09-28 12:59:08 . All other commits related to bug 474444 were merged after this one. This is not a proof that this commit is the culprit, but if the issue has appeared before this commits then is has also appeared before the ones related to bug 474444. About the colors, here is a proposal: we use the background color to draw the line around the selected item when it's not "highlighted" (part of the focus tree). Actually, any nice presentation that doesn't require to add a new color but keep the 3 states (selected/highlight, selected/non-highlighted, not selected) would be eork.
New Gerrit change created: https://git.eclipse.org/r/85574
New Gerrit change created: https://git.eclipse.org/r/85603
(In reply to Eclipse Genie from comment #10) > New Gerrit change created: https://git.eclipse.org/r/85603 This is an additional unit test reproducing the sizing issue (and failing at the moment).
Created attachment 267423 [details] Output of test Snippet @4.2, @4.3, @4.4, @4.5, @4.6, @4.7M2/M3 This bug has two parts: 1. Sizing issue, which seems like an old problem(if required we can raise a separate bug on this) 2. Focus issue is a regression introduced in 4.7 M3 via bug 507611 Just refer the output of the test snippet @4.2, @4.3, @4.4, @4.5, @4.6, @4.7M2/M3 Mickael: Can you pls take care of the focus issue ? (In reply to Eclipse Genie from comment #9) > New Gerrit change created: https://git.eclipse.org/r/85574 Above patch is abandoned as per observation in comment 5.
(In reply to Niraj Modi from comment #12) > 2. Focus issue is a regression introduced in 4.7 M3 via bug 507611 Sorry for the typo, it seems a regression of bug 474444
> 2. Focus issue is a regression introduced in 4.7 M3 via bug 507611 > Just refer the output of the test snippet @4.2, @4.3, @4.4, @4.5, @4.6, > @4.7M2/M3 See comment 5, I don't think this issue is to be perceived as a regression as it's actually fixing a usability issue. IIRC, a complete solution would require the CTabFolder to introduce a new method and color for the selected+noFocus state, which is currently missing in the CTabFolder model. The lack of such color is the root cause of both bug 474444 and the focus part of that bug. I don't think we can find any perfect solution without it.
(In reply to Mickael Istria from comment #14) > > 2. Focus issue is a regression introduced in 4.7 M3 via bug 507611 > > Just refer the output of the test snippet @4.2, @4.3, @4.4, @4.5, @4.6, > > @4.7M2/M3 > > See comment 5, I don't think this issue is to be perceived as a regression > as it's actually fixing a usability issue. > IIRC, a complete solution would require the CTabFolder to introduce a new > method and color for the selected+noFocus state, which is currently missing > in the CTabFolder model. The lack of such color is the root cause of both > bug 474444 and the focus part of that bug. I don't think we can find any > perfect solution without it. Raised bug 514164 for this enhancement in CTabFolder.
Gerrit change https://git.eclipse.org/r/85603 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=1762697759f85bf9c14f1d70dd04c6e0b941db5a
Can this one be closed now?
(In reply to Alexander Kurtakov from comment #17) > Can this one be closed now? No, as rendering without focus still broken and it depends on bug 514164. Resetting the target for now.
(In reply to Eclipse Genie from bug 389773 comment #7) > New Gerrit change created: https://git.eclipse.org/r/162590 This change (Eclipse Genie posted to the wrong ticket) should only fix the initial size problem. I have currently no plan to work on the focus/color rendering issue.
Gerrit change https://git.eclipse.org/r/162590 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=bfae5c9790e0327102cb5a33397b89aa08090059
Created attachment 283105 [details] Snippet output with bug 529742 applied Note that this snippet will be influenced by bug 529742. You can see the current state including bug 529742 in the attached screenshot.
New Gerrit change created: https://git.eclipse.org/r/164006
(In reply to Eclipse Genie from comment #22) > New Gerrit change created: https://git.eclipse.org/r/164006 Please follow bug 563822 for that.
Gerrit change https://git.eclipse.org/r/164006 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=9aa26e74a1199dbed9cd439ac84668d38b6fbd99
And a step back. :( With the revert the initial size problem is back of cause.
