Bug 150805 - [Import/Export] Incomplete validation when importing from directory
Summary: [Import/Export] Incomplete validation when importing from directory
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.2   Edit
Hardware: All All
: P5 normal (vote)
Target Milestone: ---   Edit
Assignee: Platform UI Triaged CLA
QA Contact: Prakash Rangaraj CLA
URL:
Whiteboard:
Keywords: helpwanted
Depends on:
Blocks:
 
Reported: 2006-07-17 09:55 EDT by Enrico Persiani CLA
Modified: 2009-07-23 07:37 EDT (History)
1 user (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Enrico Persiani CLA 2006-07-17 09:55:14 EDT 
The following instance methods of CheckboxTreeViewer class don't fire any CheckStateChangedEvent.

Affected methods:

setAllChecked(...)
setCheckedElements(...)
setChecked(...)

Steps to reproduce the bug:

1. File -> Import...
2. File System -> Next
3. Choose a "From directory" using the Browse... button
4. Check the root element from the left pane (the error message saying that "There are no resources currently selected for import." should disappear)
5. Press the Deselect All button and see that no error message is displayed at all!!
6. Recheck the root element twice and the error message is displayed again!!

Solution:

Add fireCheckStateChanged calls inside affected methods
Comment 1 Boris Bokowski CLA 2006-07-26 10:37:31 EDT 
This is a classic case of underspecification - the Javadoc does not explain exactly when events are fired.  Since there are many existing clients of CheckboxTreeViewer who may rely on the current implementation, I prefer to clarify the API to say that the setters don't generate any events. After that, this becomes a bug in the Import wizard.
Comment 2 Steinar Bang CLA 2007-01-30 08:18:09 EST 
While I agree that changing the current behaviour may break
for existing users of CheckboxTreeViewer (eg. eternal loops
with two mutually notifying widgets), I believe that this
should be fixed in CheckboxTreeViewer.

The reason is that the current behaviour allows two ways of
setting a checkmark (manually clicking with the mouse, and
calling #setChecked() ), where one has effects for other users
of the tree viewer, and one doesn't.

Ie. the visual effects are the same, but the end results aren't.

This just happened to me when I attempted to set check marks in
a tree viewer by selecting objects in a GEF viewer.  It seemed
to work at first... but it didn't really.  Which is why I debugged
to see what happens, and why I'm reading this bug report.

Of course, one can always subclass, which is what I'll do for now.
Comment 3 Steinar Bang CLA 2007-01-30 08:52:15 EST 
Comment to my previous comment:

On closer thought, it would probably be wrong to introduce potential eternal 
loops (or stack overflows) to fix this bug... probably...?

I'm not sure what the right answer is.
Comment 4 Boris Bokowski CLA 2007-01-30 09:26:57 EST 
Fixed the underspecification problem by adding the following sentence to the Javadoc where appropriate:
"Does not fire events to check state listeners."

Changing the title and moving back to the inbox because the remaining bug is in the import/export code.
Comment 5 Tod Creasey CLA 2007-06-19 14:37:24 EDT 
We currently don't have any plan to work on this but we would accept a patcy
Comment 6 Susan McCourt CLA 2009-07-09 19:10:58 EDT 
As per http://wiki.eclipse.org/Platform_UI/Bug_Triage_Change_2009
Comment 7 Prakash Rangaraj CLA 2009-07-23 07:37:03 EDT 
Followed the steps. At step 5, the error is shown after clicking Deselect All. Probably got fixed with some other bug