| Summary: | Table and Tree with SWT.SINGLE should not allow user to deselect item | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Markus Keller <markus.kell.r> | ||||
| Component: | SWT | Assignee: | Scott Kovatch <skovatch> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | daniel_megert, eclipse.felipe, schulz.johan, Silenio_Quarti | ||||
| Version: | 3.7 | ||||||
| Target Milestone: | 3.7 M3 | ||||||
| Hardware: | PC | ||||||
| OS: | Mac OS X | ||||||
| Whiteboard: | |||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 325223 | ||||||
| Attachments: |
|
||||||
|
Description
Markus Keller
> causing trouble (bug 325223).
In 3.7 M1, this was not an issue, since the SWT.Selection event didn't get through when the table's shell didn't have focus (but MouseDown/MouseUp were delivered). Now, the selection event is also delivered in our scenario.
We'd like to see this fixed for M2 if possible. If not, please let us know asap, so that we can add a workaround on our side. Bug 88288 was closed as WONTFIX, but I disagree with the analysis that 'once a Table has selection it always has a selection' is (or should be) platform behavior. That's water under the bridge, I guess. This is easy to fix, however. [NSTableView setAllowsEmptySelection:NO] should do the job. Give me an hour or so to test Table and Tree. (In reply to comment #3) +1. It's always bad if platform differences shine through SWT APIs. > This is easy to fix, however. [NSTableView setAllowsEmptySelection:NO] should > do the job. Give me an hour or so to test Table and Tree. I didn't try it with setAllowsEmptySelection:NO, but the documentation says you cannot even set an empty selection programmatically any more. But on Windows and GTK, that's still possible (and a SINGLE table has nothing selected initially). Created attachment 178861 [details]
Fix for Tree, List, & Table
Implemented a delegate method that allows you to modify the selection. I want to check with Silenio if it should still be possible to clear the selection programmatically, though.
We should fix this after 3.7 M2 given that we need more testing. (In reply to comment #4) > I didn't try it with setAllowsEmptySelection:NO, but the documentation says you > cannot even set an empty selection programmatically any more. But on Windows > and GTK, that's still possible (and a SINGLE table has nothing selected > initially). Right -- I hit that early on and wound up taking another path. The attached patch works for user clicks and programmatic selection and deselection. (In reply to comment #6) > We should fix this after 3.7 M2 given that we need more testing. Agreed. Markus, please release the workaround for M2. Fixed > 20100920 I took a closer look at the patch. Isn't the NSIndexSet allocated in selectionIndexesForProposedSelection() leaking? (In reply to comment #10) > I took a closer look at the patch. Isn't the NSIndexSet allocated in > selectionIndexesForProposedSelection() leaking? Yep, you're right. They need an autorelease before being returned. Fixed. I've reverted the workaround in Platform Text. Scott, I don't have a Mac at hand, can you verify in tomorrows build (N20100923-2000) that bug 325223 is gone? (In reply to comment #12) > Scott, I don't have a Mac at hand, can you verify in tomorrows build > (N20100923-2000) that bug 325223 is gone? Okay, if you tell me where I can find it in the UI. I looked in the 3.7M2 N&N but didn't see any new features that look like what's going on in 325223. (In reply to comment #13) > (In reply to comment #12) > > Scott, I don't have a Mac at hand, can you verify in tomorrows build > > (N20100923-2000) that bug 325223 is gone? > > Okay, if you tell me where I can find it in the UI. Sure. Paste the following code into the Package Explorer: -- %< --- class C { public String toString() { return ""; } } -- %< --- Now click on "toString" while holding down the Command modifier. This should bring up a little shell with two options. Click on one of them to jump to that method. In I20100914-0100 (had bug) it did not jump but simply deselect the item. > I looked in the 3.7M2 N&N but didn't see any new features that look like > what's going on in 325223. It's not a new feature but one that got broken by changes in SWT. Scott, were my steps clear enough to verify it? Yes, sorry about that. Eclipse.org downloads have been especially slow lately. I verified it with N20100926-2000 just now. Part for the fix for this problem (according to CVS log) was to call translateTraversal() from inside Control#insertText(), this does not show in the patch and does not (apparently) relates to problem described in here. Maybe the code was released with the wrong comment in it. Anyhow, this code is causing bug 343093 (input method is broken for single line text). (In reply to comment #17) > Part for the fix for this problem (according to CVS log) was to call > translateTraversal() from inside Control#insertText(), this does not show in > the patch and does not (apparently) relates to problem described in here. Maybe > the code was released with the wrong comment in it. Anyhow, this code is > causing bug 343093 (input method is broken for single line text). The log in the bugzilla is wrong, it reads: 325230 - check for translateTraversal in insertText. but it should be: 325222 - check for translateTraversal in insertText. |