Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 394016

Summary: Ctrl+PageDown doesn't work multiple times in Classic theme
Product: [Eclipse Project] Platform Reporter: Markus Keller <markus.kell.r>
Component: SWTAssignee: Carolyn MacLeod <carolynmacleod4>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: carolynmacleod4, daniel_megert, emoffatt, gheorghe, pwebster, Silenio_Quarti
Version: 4.3   
Target Milestone: 4.2.2   
Hardware: PC   
OS: Windows 7   
Whiteboard:

Description Markus Keller CLA 2012-11-09 16:48:25 EST
I20121106-0800

I try to switch two or more tabs to the right, so I press Ctrl+PageDown and quickly repeat PageDown.

This rarely brings me to the correct editor tab. Often, the first editor is active in the end (also if start in the middle of the screen).

If you do it slowly, you can see that the Minimize button is focused at one point. That could be part of the explanation for this bug.
Comment 1 Carolyn MacLeod CLA 2012-11-12 10:13:02 EST
FYI, the following SWT-only CTabFolder snippet works fine every time.
(Tabs are numbered so that it is easy to know if you arrived at the correct tab no matter how many ctrl+page downs you quickly type).

import org.eclipse.swt.*;
import org.eclipse.swt.widgets.*;
import org.eclipse.swt.layout.*;
import org.eclipse.swt.custom.*;

public class CTabFolderManyTabsTest {

	public static void main(String[] args) {
		Display display = new Display();
		Shell shell = new Shell(display);
		shell.setLayout(new FillLayout());
		
		CTabFolder tabFolder = new CTabFolder(shell, SWT.FLAT);
		for (int i = 0; i < 12; i++) {
			CTabItem item = new CTabItem(tabFolder, SWT.NULL);
			item.setText("Tab " + i);
			Text itemText = new Text(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();
		}
	}
}
Comment 2 Carolyn MacLeod CLA 2012-11-12 10:15:31 EST
(Works fine with:
   StyledText itemText = new StyledText(tabFolder, SWT.MULTI);
also... which would slightly more closely match the editor part).
Comment 3 Carolyn MacLeod CLA 2012-11-12 10:20:21 EST
Wow - you say it rarely works for you, but I tried to see the failure in my running eclipse, and I always hit the correct tab. I am only doing a maximum of 6 ctrl+tabs in a row, and there are only 6 visible editor tabs. I am running:
Version: 4.3.0
Build id: I20121031-2000
Comment 4 Markus Keller CLA 2012-11-12 11:01:44 EST
Sorry, I only wrote "Classic theme" in the subject. Did you really try it with the Classic theme?

When I perform the steps in bug 394109 and then switch to the Classic theme and restart, I can often reproduce. When I enable the Breadcrumb and then give focus back to the editor (e.g. via Esc), then it rarely works correctly.

> If you do it slowly, you can see that the Minimize button is focused at one
> point.
If the overflow chevron is shown, then that one also quickly gets focus.

I guess the bug is not in SWT, but in some async event handling or firing in E4.
Comment 5 Dani Megert CLA 2012-11-12 11:03:13 EST
(In reply to comment #3)
> Wow - you say it rarely works for you,

Same here, but I need to do it really fast to break it and you need to be in the 'Classic' theme.
Comment 6 Carolyn MacLeod CLA 2012-11-12 12:54:27 EST
Ah - I see the failure now. Sorry about that - I haven't used Classic theme in a long time.

For what it's worth, here's a newer little SWT-only snippet that emulates "Classic" theme (with swoosh-style tabs, selected tab gradient, chevron, min and max). I cannot get this snippet to fail.

import org.eclipse.swt.*;
import org.eclipse.swt.widgets.*;
import org.eclipse.swt.layout.*;
import org.eclipse.swt.custom.*;
import org.eclipse.swt.graphics.*;

public class CTabFolderManyTabsTest {

	public static void main(String[] args) {
		Display display = new Display();
		Shell shell = new Shell(display);
		shell.setLayout(new FillLayout());
		
		CTabFolder tabFolder = new CTabFolder(shell, SWT.NONE);
		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();
		}
	}
}
Comment 7 Markus Keller CLA 2012-11-12 13:16:07 EST
> For what it's worth, here's a newer little SWT-only snippet that emulates
> "Classic" theme.

Thanks, I could use that for bug 377113...
Comment 8 Carolyn MacLeod CLA 2012-11-12 15:07:47 EST
You're welcome.  :)

Here's another SWT-only snippet that emulates Eclipse's "asynchronously force focus to the CTabItem's control when the CTabItem is activated or selected" UI behavior. Not sure if this snippet is helpful... it still does not fail no matter how fast I try to type ctrl+page up/down. So something more is going on behind the scenes in Eclipse that is causing this problem - I don't think it's in CTabFolder or SWT.

import org.eclipse.swt.*;
import org.eclipse.swt.custom.*;
import org.eclipse.swt.events.*;
import org.eclipse.swt.graphics.*;
import org.eclipse.swt.layout.*;
import org.eclipse.swt.widgets.*;

public class CTabFolderManyTabsTest {
	static boolean forceFocusOnActivate = true;
	static boolean forceFocusOnSelection = true;

    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);
        }
		tabFolder.addTraverseListener(new TraverseListener() {
			public void keyTraversed(TraverseEvent e) {
				switch (e.detail) {
					case SWT.TRAVERSE_ARROW_NEXT:
					case SWT.TRAVERSE_ARROW_PREVIOUS:
						forceFocusOnActivate = false;
						forceFocusOnSelection = false;
						break;
				}
			}
		});
		tabFolder.addSelectionListener(new SelectionAdapter() {
			public void widgetSelected(SelectionEvent e) {
				if (forceFocusOnSelection) focusOn(e.widget);
				forceFocusOnSelection = true;
			}
		});
		tabFolder.addListener(SWT.Activate, new Listener() {
			public void handleEvent(Event e) {
				if (forceFocusOnActivate) focusOn(e.widget);
				forceFocusOnActivate = true;
			}
		});
		tabFolder.addKeyListener(new KeyAdapter() {
			public void keyPressed(KeyEvent e) {
				if (e.keyCode == SWT.CR) {
					focusOn(e.widget);
					forceFocusOnSelection = true;
				}
			}
		});

        shell.pack();
        shell.open();
        while (!shell.isDisposed()) {
            if (!display.readAndDispatch()) display.sleep();
        }
    }
    
	// Emulate Eclipse's "asynchronously force focus to CTabItem's control" behavior
	static void focusOn(final Widget widget) {
		widget.getDisplay().asyncExec(new Runnable() {
			public void run() {
				CTabFolder tf = (CTabFolder) widget;
				CTabItem ti = tf.getSelection();
				if (ti != null) {
					Control c = ti.getControl();
					if (c != null && !c.isDisposed()) {
						c.setFocus();
					}
				}
			}
		});
	}
}
Comment 9 Markus Keller CLA 2012-11-13 06:17:35 EST
(In reply to comment #8)
I also couldn't reproduce with this snippet, but I saw that the focus flashing on the Minimize button also happens with this snippet. To make it more apparent, change the asyncExec to timerExec(2000, ...).

And the flashing on the chevron can be seen if you add a Thread.sleep(1000); at the beginning of focusOn(..) and then press Ctrl+PageDown twice.

Neither of these problems shows up in R3_8_maintenance.
Comment 10 Carolyn MacLeod CLA 2012-11-14 10:01:04 EST
Fixed in master:
http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=bc7e1bc085c7d5faaae0b0da826b492f201e635a

We looked at this, and discussed the "interesting focus issues" that were going on, and decided that the max, min, and chevron buttons really should not take focus. There are existing keyboard shortcuts for these buttons:
- Ctrl+E for the chevron
- Ctrl+M for Maximize/Restore
- I don't see one for Minimize... I will open a bug to have one added

I temporarily set the target milestone to 4.3M4, but we will decide in the next few days (after trying it in a build) whether to backport to 4.2.2 (it does not need to be backported to 3.8.2 because max/min/chevron were all drawn back then).

Leaving open to consider backporting.
Comment 11 Markus Keller CLA 2012-11-15 13:57:44 EST
(In reply to comment #10)
> Fixed in master:

Nope, I can still reproduce the issue that the first tab is wrongly selected when I traverse tabs. An easier-to-see variant is to open enough editors so that the chevron appears, then go to the last tab and press Ctrl+PageUp a few times. When the drow-down opens, you've hit the bug.

> not take focus. There are existing keyboard shortcuts for these buttons:

These shortcuts only work in Eclipse (and maybe RCP application). Shouldn't all SWT apps be accessible?

> - I don't see one for Minimize... I will open a bug to have one added

No need to do that. There's already a command and a menu item (in Window > Navigation) for this. There's no default shortcut, and there shouldn't be one, because this is not an often-used action. In 3.8, you can also minimize via Alt+- (regression in E4, bug exists).
Comment 12 Carolyn MacLeod CLA 2012-11-15 16:09:00 EST
> Nope, I can still reproduce the issue that the first tab is wrongly selected
> when I traverse tabs.

I can't get the failure case to happen.

> An easier-to-see variant is to open enough editors so that the chevron appears,
> then go to the last tab and press Ctrl+PageUp a few times. When the drop-down 
> opens, you've hit the bug.

The drop-down doesn't open for me. Is that a special setting in Eclipse, maybe?

> These shortcuts only work in Eclipse (and maybe RCP application).
> Shouldn't all SWT apps be accessible?

Yes, and I need to document in CTabFolder how to add keyboard accessibility for maximize/restore/[minimize]. (CTabFolder cannot do these on its own).

I don't think CTabFolder needs a keyboard shortcut to open its "native" chevron menu because Ctrl+PageUp/PageDown cycles through all of the tabs anyhow, and there is no advantage to be gained by dropping down the chevron menu.
I could also document how Eclipse shows the nice filtered list when the chevron is dropped down (by either mouse or keyboard), because the filtered list is useful for keyboard users.

> No need to do that. There's already a command and a menu item
> (in Window > Navigation) for this.

Thanks! Sounds good.
Comment 13 Markus Keller CLA 2012-11-16 07:03:55 EST
(In reply to comment #12)
> The drop-down doesn't open for me. Is that a special setting in Eclipse,
> maybe?

The drop-down should always open in Eclipse when you're on the first or last visible tab and then try to traverse to the next tab outside of the visible tab set. I don't think there's a setting to configure that.

Again, it's much easier to reproduce if you slow down the Java editor a bit by enabling the Breadcrumb.

> I don't think CTabFolder needs a keyboard shortcut to open its "native"
> chevron menu because Ctrl+PageUp/PageDown cycles through all of the tabs
> anyhow, and there is no advantage to be gained by dropping down the chevron
> menu.

I agree about the chevron. But it's a bit strange that additional topRight actions in the tab bar are reachable via Ctrl+Tab, but Minimize/Maximize are not. See e.g. in the CustomControlExample. There, you also see that the focus currently goes to topRight toolbar when you press Ctrl+PageDown.
Comment 14 Carolyn MacLeod CLA 2012-11-19 11:09:22 EST
> The drop-down should always open in Eclipse when you're on the first or last
> visible tab and then try to traverse to the next tab outside of the visible
> tab set.

Hmmm... I see what you mean. This behavior does not happen now with the code I released in comment 10, which is why I didn't see it. The user doesn't get the chevron menu with Ctrl+PageDown any more. Now, Ctrl+PageDown/PageUp cycles through the tabs. To get the chevron menu, use Ctrl+E. This is probably more "expected" behavior for the non-visual user.

> But it's a bit strange that additional topRight actions in the tab bar are
> reachable via Ctrl+Tab, but Minimize/Maximize are not.

Yikes, you are right. This is very strange - I would call it a bug. Focus should not go to the top right toolbar on Ctrl+PageDown.  Users are supposed to type Tab, Shift+Tab, Ctrl+Tab, or Ctrl+Shift+Tab to navigate to the toolbar. I will think about this some more.
Comment 15 Markus Keller CLA 2012-11-19 12:21:57 EST
(In reply to comment #14)
In Eclipse, the editor menu still shows up on Ctrl+PageDown/PageUp for me (comment 10 doesn't change this). Note that this is an Eclipse-only customization that you don't get in pure SWT examples.

> > But it's a bit strange that additional topRight actions in the tab bar are
> > reachable via Ctrl+Tab, but Minimize/Maximize are not.
> 
> Yikes, you are right. This is very strange - I would call it a bug. Focus
> should not go to the top right toolbar on Ctrl+PageDown.

That's not what I meant, but you're right, that's another bug. In the CustomControlExample, I see this independent of comment 10 (but only if I start with the focus in the tab item's content control, not when focus is on the tab itself. In the SDK, it's not visible, since the focus is always set back into the tab content.

> Users are supposed to type Tab, Shift+Tab, Ctrl+Tab, or Ctrl+Shift+Tab to
> navigate to the toolbar.

Yes, that's what I meant. But I also consider the Minimize/Maximize buttons to be part of the toolbar, so treating Min/Max different from CTabFolder#setTopRight(..) items is strange.
Comment 16 Carolyn MacLeod CLA 2012-11-26 16:24:52 EST
New commit, overrides the one in comment 10.

http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=b31a2c4fef115f61f56aab574a9b54fe849df0f4

This one lets min/max and chevron take focus again (so that I don't get in trouble with the accessibility people <grin>). It fixes the strange toolbar-gets-focus-on-page-traversal behavior discussed in comment 14 and comment 15. And I believe it fixes the original problem of getting lost when traversing pages very quickly. (At least, I tried it many times with 10 ctrl+pagedowns in a row, and I could not make it fail).

Please try this in tomorrow's integration build, when it is available.
Note that I have only tried it on Windows so far - I will try it on Mac and Linux tomorrow.

I am leaving this bug open for now - please let me know what you think.
Comment 17 Markus Keller CLA 2012-11-27 07:22:08 EST
(In reply to comment #16)
The focus flashing on the toolbar button is gone, and the buttons are accessible via Ctrl+Tab. The fix is good for these problems.

However, I don't see a change regarding the wrong tab selection on quick Ctrl+PageUp/Downs. I debugged the problem with a breakpoint in CTabFolder#setSelection(int) with condition "index == 0" and found two problems:

- CTabFolder#setSelection(int) sets "selection.showing = false;". In the next page traversal, CTabFolder#onPageTraversal(Event) sometimes doesn't find the selected item, because its 'showing' field is still 'false'.

- When I revert commit c68f62c9983c4bd3a099b9805f372a1acbf5843f (bug 389773), then I can't reproduce the problem. Problem is probably the asyncExec.

It looks like a timing issue, and it's probably easier to reproduce if you enable as many "Link with Editor" views as possible to make editor switching slower (e.g. enable linking in Package Explorer and the History view).
Comment 18 Markus Keller CLA 2012-11-27 07:57:46 EST
There's a standalone snippet that shows the problem:

import org.eclipse.swt.*;
import org.eclipse.swt.custom.*;
import org.eclipse.swt.events.*;
import org.eclipse.swt.layout.*;
import org.eclipse.swt.widgets.*;

public class CTabFolderBug394016 {

    public static void main(String[] args) {
        Display display = new Display();
        Shell shell = new Shell(display);
        shell.setLayout(new FillLayout());

        CTabFolder tabFolder = new CTabFolder(shell, SWT.BORDER);
        tabFolder.setSimple(false);
        tabFolder.setMaximizeVisible(true);
        tabFolder.setMinimizeVisible(true);
        tabFolder.setMRUVisible(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);
        }
        
        tabFolder.addSelectionListener(new SelectionAdapter() {
            @Override
            public void widgetSelected(SelectionEvent e) {
                try {
                    Thread.sleep(200);
                } catch (InterruptedException e1) {
                    e1.printStackTrace();
                }
            }
        });

        shell.setSize(300, 100);
        shell.open();
        while (!shell.isDisposed()) {
            if (!display.readAndDispatch()) display.sleep();
        }
    }
}

The factors are setMRUVisible(true) and a slow SelectionListener. Note that ArrowLeft/Right have the same problems.
Comment 19 Carolyn MacLeod CLA 2012-11-29 09:52:18 EST
Thanks for the snippet, Markus - that really helped.

Here's a new fix that updates without async for keyboard events.
Released to master:
http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=2eb0d57b46ef2fa0c4a9cfafa527354a559190cd

Please test in a build and let me know if you still have any problems.
After your ok, I will backport the fix to 4.2 and close this bug.
Comment 20 Bogdan Gheorghe CLA 2012-11-29 09:57:03 EST
Switching Target to 4.2.2, so we don't forget to backport this.
Comment 21 Markus Keller CLA 2012-12-17 13:48:13 EST
Thanks. I tried hard to break it, but all problems mentioned in this bug are indeed fixed now.

I still don't understand why CTabFolder#setSelection(int) sets "selection.showing = false;", but that's been in for 7 years now, so it can't be that bad (and taking it out breaks the rendering of the close button...).
Comment 22 Eric Moffatt CLA 2012-12-17 14:02:27 EST
Markus, should I close this as FIXED then ??
Comment 23 Markus Keller CLA 2012-12-17 14:13:10 EST
(In reply to comment #22)
> Markus, should I close this as FIXED then ??

Nope, SWT will use this bug to backport to 4.2.2, see comment 20.
Comment 24 Markus Keller CLA 2012-12-17 14:14:07 EST
Sorry, I only now saw that the bug's component was wrongly set to UI.
Comment 25 Eric Moffatt CLA 2012-12-17 14:22:05 EST
OK, thanks !
Comment 27 Markus Keller CLA 2013-01-17 10:19:55 EST
Verified in M20130116-1800 and master.