Bug 348537

Comment 1 Curtis Windatt 2011-06-07 17:21:09 EDT

Should be an easy fix.  Would you consider contributing a patch?

Comment 2 Wim Jongman 2011-06-08 06:48:14 EDT

Sure! 3.7 target?

Comment 3 Remy Suen 2011-06-08 08:53:39 EDT

(In reply to comment #2)
> Sure! 3.7 target?

3.7 is closed for development right now. We do not plan to contribute anymore code to 3.7 unless the PMC deems an issue to be stop-ship.

Comment 4 Wim Jongman 2011-06-08 18:15:43 EDT

note to self: ExportPackageSection is a class of interest

Comment 5 Curtis Windatt 2011-09-15 13:03:47 EDT

ping Wim. any progress?

Comment 6 Wim Jongman 2011-09-22 13:25:32 EDT

Analysis:

When the Add button is pressed in the Runtime tab of the Manifest Editor, the handleAdd() method of ExportPackageSection is called. It calls dialog.create() on line 474. This will call create() on AbstractElementListSelection which is part of o.e.ui.workbench. 

create() will call handleEmptyList() and disable the dialog elements if the initial list is empty. The AELS was not designed to suddenly receive new elements and enable the dialog elements again and this is what happens in the concrete class ConditionalListSelectionDialog which subclasses AELS.

So we need changes in two places:

in AELS:

    /**
     * Handles empty list by disabling widgets.
     */
    protected void handleEmptyList() {
    	boolean enable = fFilteredList.isEmpty()?false:true;
        fMessage.setEnabled(enable);
        fFilterText.setEnabled(enable);
        fFilteredList.setEnabled(enable);
        updateOkState();
    }

in CLSD we need to add a call to this method when the non-java package button is flipped.

	protected Control createDialogArea(Composite parent) {
		Composite comp = (Composite) super.createDialogArea(parent);
		int size = ((fElements != null) ? fElements.length : 0) + ((fConditionalElements != null) ? fConditionalElements.length : 0);
		final Object[] allElements = new Object[size];
		int conditionalStart = 0;
		if (fElements != null) {
			System.arraycopy(fElements, 0, allElements, 0, fElements.length);
			conditionalStart = fElements.length;
		}
		if (fConditionalElements != null)
			System.arraycopy(fConditionalElements, 0, allElements, conditionalStart, fConditionalElements.length);

		final Button button = new Button(comp, SWT.CHECK);
		Assert.isNotNull(fButtonText);
		button.setText(fButtonText);
		button.addSelectionListener(new SelectionAdapter() {
			public void widgetSelected(SelectionEvent e) {
				if (button.getSelection())
					setListElements(allElements);
				else
					setListElements(fElements);

	======>			handleEmptyList();
			}
		});
		return comp;
	}

Comment 7 Wim Jongman 2011-09-22 13:40:26 EDT

Ah, no it can be more elegant: whenever setListElements([]) is called the status of the empty list must be calculated again so everything can stay in AbstractElementListSelectiondialog.  


Index: Eclipse UI/org/eclipse/ui/dialogs/AbstractElementListSelectionDialog.java
===================================================================
RCS file: /cvsroot/eclipse/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/dialogs/AbstractElementListSelectionDialog.java,v
retrieving revision 1.20
diff -u -r1.20 AbstractElementListSelectionDialog.java
--- Eclipse UI/org/eclipse/ui/dialogs/AbstractElementListSelectionDialog.java	13 Nov 2008 08:25:53 -0000	1.20
+++ Eclipse UI/org/eclipse/ui/dialogs/AbstractElementListSelectionDialog.java	22 Sep 2011 17:37:56 -0000
@@ -176,6 +176,7 @@
     protected void setListElements(Object[] elements) {
         Assert.isNotNull(fFilteredList);
         fFilteredList.setElements(elements);
+		handleEmptyList();
     }
 
     /**
@@ -455,9 +456,10 @@
      * Handles empty list by disabling widgets.
      */
     protected void handleEmptyList() {
-        fMessage.setEnabled(false);
-        fFilterText.setEnabled(false);
-        fFilteredList.setEnabled(false);
+		boolean enable = fFilteredList.isEmpty() ? false : true;
+		fMessage.setEnabled(enable);
+		fFilterText.setEnabled(enable);
+		fFilteredList.setEnabled(enable);
         updateOkState();
     }

Comment 8 Curtis Windatt 2011-09-30 18:14:28 EDT

Moving to Platform UI as the proposed change is in their code. 

If you design a workaround for the PDE code that creates the selection dialog I will look at it.

Comment 9 Wim Jongman 2012-01-25 12:53:37 EST

Hi Platform, any chance that someone can take a look at this patch?

Comment 10 Michael Rennie 2012-02-23 11:59:52 EST

I took a look at the patch and made a few minor changes:

1. updated the doc on the handleEmptyList method to remove the blurb about being used only in the #open method call

2. added a new API method handleElementsChanged() that is now called from #setListElements. The reason for this is so that we do not changed the default API behavior of the handleEmptyList method, and clients can override the new method specifically in response to the list being changed.

I pushed these changes to my github branch:

https://github.com/mrennie/eclipse.platform.ui/tree/mrennie/bug348537

Comment 11 Eric Moffatt 2012-02-28 10:54:40 EST

Pushed in >20120228.

commit 207cb1a4b974d17672085f4e9bf590107e5cfb97  (master)

commit 1db6582c0f432727871148ea19642c8920e5b97a  (R3_Development)

Beauty, thanks Mike !!

Comment 12 Dani Megert 2012-02-29 04:45:39 EST

(In reply to comment #11)
> Pushed in >20120228.
> 
> commit 207cb1a4b974d17672085f4e9bf590107e5cfb97  (master)
> 
> commit 1db6582c0f432727871148ea19642c8920e5b97a  (R3_Development)
> 
> Beauty, thanks Mike !!

Not really ;-). The @since tag in 3.8 must be 3.8 and the same should be used in the 4.2 stream for APIs added in 3.8. If you had API Tools enabled and a baseline installed - which everyone should (http://dev.eclipse.org/mhonarc/lists/eclipse-dev/msg09310.html), then you should have seen an error on the project.

I've fixed the @since tag in 3.8 and 4.2.

Comment 13 Curtis Windatt 2012-03-13 15:29:32 EDT

Verified

Version: 3.8.0
Build id: I20120312-1800

Version: 4.2.0
Build id: I20120313-0610