Bug 341134 - Tree#selectAll() should only select visible items (those without any collapsed parent)
Summary: Tree#selectAll() should only select visible items (those without any collapse...
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 3.7   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 3.7 M7   Edit
Assignee: Felipe Heidrich CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 341599
  Show dependency tree
 
Reported: 2011-03-28 12:32 EDT by Markus Keller CLA
Modified: 2011-04-28 11:00 EDT (History)
3 users (show)

See Also:


Attachments
patch (1.84 KB, patch)
2011-04-01 16:40 EDT, Felipe Heidrich CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2011-03-28 12:32:19 EDT
I20110322-0800

Tree#selectAll() should only select visible items (i.e. roots and items whose parents are expanded and visible). Currently, it selects all items in the Tree, which is unexpected. The problem can be seen with Snippet61 if you add this listener and then press "A":

tree.addListener(SWT.KeyDown, new Listener() {
	public void handleEvent(Event e) {
		if (e.keyCode == 'a') {
			e.doit= false;
			System.out.println("selectAll");
			tree.selectAll();
			tree.notifyListeners(SWT.Selection, new Event());
		}
	}
});

On GTK, only visible items are selected (as expected).

This is a problem in the SDK, which calls selectAll() on Ctrl+A, so we see the wrong selections in many places. An example where this gets really weird is the tree in the Edit JRE dialog from the Installed JREs preference page: Right after opening the dialog, I can press Ctrl+A and then click Javadoc Location... . But after I've expanded and collapsed the first tree item, Ctrl+A disables the button.
Comment 1 Felipe Heidrich CLA 2011-03-28 13:00:35 EDT
the javadoc says:
"Selects all of the items in the receiver."

Silenio, what do you think is the correct behaviour ?

what happens when you call select (TreeItem) ? I bet the item is select even when it is not visible....
Comment 2 Remy Suen CLA 2011-03-28 13:15:30 EDT
Personally, I think selecting everything is correct. If you just read the method's name it doesn't even hint at the visibility state of its items so I don't see why it should be considered.
Comment 3 Markus Keller CLA 2011-03-28 13:38:34 EDT
(In reply to comment #2)
> Personally, I think selecting everything is correct.

That would mean Platform UI needs a special case for Tree in SelectAllHandler, since a user doesn't expect to have invisible items selected.

Also note that selected items lose their selection when the parent is collapsed, so this selection of hidden items really looks like an accident.


(In reply to comment #1)
> what happens when you call select (TreeItem) ? I bet the item is select even
> when it is not visible....

It depends... . The first item in the selection gets automagically revealed, but the others stay invisible (bug 158734).

tree.addListener(SWT.KeyDown, new Listener() {
	public void handleEvent(Event e) {
		if (e.keyCode == 'b') { // select 1 item
			e.doit= false;
			System.out.println("select 0 0");
			tree.setSelection(tree.getItem(0).getItem(0));
			tree.notifyListeners(SWT.Selection, new Event());
		}
	}
});
tree.addListener(SWT.KeyDown, new Listener() {
	public void handleEvent(Event e) {
		if (e.keyCode == 'c') { // select 2 items
			e.doit= false;
			System.out.println("select 0 0, 1 1");
			tree.setSelection(new TreeItem[] {
					tree.getItem(0).getItem(0),
					tree.getItem(1).getItem(1) });
			tree.notifyListeners(SWT.Selection, new Event());
		}
	}
});
Comment 4 Remy Suen CLA 2011-03-28 14:00:03 EDT
There was a misunderstanding here. When Markus said "visible" I read it as "visible to the end user", that being, the tree items may be there but not actually on the screen because you had to _scroll_ to see them. It seems what Markus was referring to was child items of a parent that was collapsed.

(In reply to comment #3)
> Also note that selected items lose their selection when the parent is
> collapsed, so this selection of hidden items really looks like an accident.

I have seen this on Windows XP and it is indeed quite weird.
Comment 5 Silenio Quarti CLA 2011-03-31 14:44:46 EDT
I guess it makes sense to select only the visible items. I do not think it is possible on the other platforms to select "invisible" items. Their parents need to be expanded. Let's change windows and make this consistent.
Comment 6 Markus Keller CLA 2011-04-01 06:26:12 EDT
Thanks.

When you fix selectAll(), please take care that the fix doesn't break the scenario from bug 158734, i.e. that Tree#setSelection(TreeItem[]) still selects all the passed items.

On Windows, setSelection(*) only reveals the first item, but the others are still selected and can be used by actions (e.g. Delete, Organize Imports, ...). That the items are not revealed is a minor problem, but if they were not selected any more because they are not visible, that would break some workflows.
Comment 7 Felipe Heidrich CLA 2011-04-01 16:40:24 EDT
Created attachment 192388 [details]
patch
Comment 8 Felipe Heidrich CLA 2011-04-01 16:44:51 EDT
(In reply to comment #6)
> Thanks.
> When you fix selectAll(), please take care that the fix doesn't break the
> scenario from bug 158734, i.e. that Tree#setSelection(TreeItem[]) still selects
> all the passed items.
> On Windows, setSelection(*) only reveals the first item, but the others are
> still selected and can be used by actions (e.g. Delete, Organize Imports, ...).
> That the items are not revealed is a minor problem, but if they were not
> selected any more because they are not visible, that would break some
> workflows.

the fix for this bug only affects callers of selectAll().

Fixed in HEAD