Description Koen van Dijken 2007-02-12 12:57:20 EST

Build ID: I20061102-1715
GEF 3.2.0.v20060626

Steps To Reproduce:
In our editor it's easy to reproduce, but in LogicEditor I could not reproduce it

1. Select an object in the editor
2. An action derived from SelectionAction is enabled by selecting the object in step 1.
3. Switch to a different workbenchpart for which this action is not available. The action disables
4. switch back to the editor

The action does not get the right selection to base its enablement from. The reason is a wrong construct in SelectionAction.update(). It is implemented as follows:


public void update() {
  if (provider != null)
    setSelection(provider.getSelection());
  else
    setSelection(
      getWorkbenchPart()
        .getSite()
        .getWorkbenchWindow()
        .getSelectionService()
        .getSelection());
}


but should be implemented as

public void update() {
  if (provider != null)
    setSelection(provider.getSelection());
  else
    setSelection(
      getWorkbenchPart()
        .getSite()
        .getSelectionProvider()
        .getSelection());
}



With the wrong construct there is a risk that the selection obtained from the selectionService is the selection from the previous part, and not of the just activated part. This is because the selection service updates its own selection as part of the part-change-listening proces (setActivepart), the same proces from which update() is called. There is no predefined order in which the listeners to the partChanged event will be called. When update() is called before setActivepart of the selectionService is called, the wrong selection will be returned.

Also, it's more logical in SelectionAction.update() to 

(a) derive the current selection from the provider in use (be it the alternative selection source (see setSelectionProvider) or the provider of the active part)

than

(b) in one case use the alternative selection source, other case ask the selectionservice.


With the current construct of SelectionAction.update() it is required that update() is called from a selectionListener registered in the selectionService (in LogicEditor it is the editor which is registered and calls update()). When update() is called from the selectionProvider of the editor, this construct fails. We do not really want to call update() from the editor, because the editor should not concern itself with its actions, for that we have the editorActionBarContributor (see help on org.eclipse.ui.editors). LogicEditor is not following Eclipse guidelines in this aspect.

Hope I make myself clear......

More information: