@kalali Okay? 😄

I also tested this yesterday and it worked. However, when setting the data manager selection by another plugin (e.g. by using 'SynchronizeDataManagerSelection'), the selection of my own plugin (in this case the Render Window Manager) is lost.
I tested some things and came up with a solution: In 'QmitkDataManagerView::NodeSelectionChanged' I changed:
'node->SetBoolProperty("selected", true/false);'
to
'node->SetSelected(true/false);'
I need to investigate further on this but it seems that the setting of the bool property somehow triggers something that clears the current selection of my plugin. Using 'SetSelected' does only reset the value of the property without creating a new property.

Using both fixes of Miklos and my proposed change I can get the Render Window Manager to change the selection of the data manager according to the RWM selection and also notify all observer!

node->SetBoolProperty("selected", true/false); and node->SetSelected(true/false); are almost exactly the same.

SetBoolProperty creates a new BoolProperty and sets it to the property list of the node. The property list will send a modified event, and the node is listening to the modified events of its property list, and send its own modified event after any change.

SetSelected does not create a new bool property (if there is one), and changes the value of the current property but then sends the modified event for the node manually. The only difference is that there is no modified event for the property list, just for the node.

It would be strange if this did matter, though.

That's exactly how I understood the difference. And I find it strange, too. I have to take a closer look into the mechanism and my plugin, maybe I missed something.
However, your suggestions for consistency seems very helpful, thank you.

Another important difference is, that if the selection of a node has already been set (e.g. to false by your new consistency suggestion) and the node's "selected" property is now set again to the same value, no modified event is sent.

As I see, PropertyList::SetProperty() does an equality check, and returns early if the parameter is equal to the current value. No modified event is sent, either.

By the way, I might have discovered another bug in this class. The documentation of the class says that the model has to support QmitkDataNodeRole and return a mitk::DataNode::Pointer object for each QModelIndex in the selection.

In QmitkDataNodeSelectionProvider::SetSelection(), however, not this role is used but QmitkDataNodeRawPointerRole when the selected nodes are searched for in the model. With the data manager this works because its model (QmitkDataStorageTreeModel) supports both roles.

But QmitkAbstractView uses QmitkDataNodeItemModel that is an internal code, not even exported. This class does not override the data() function.

Yes, you're right, I overlooked this. However, the 'nodes = this->GetCurrentSelection();' contains no elements in my case, when using 'node->SetBoolProperty("selected", false);'. It contains the selected node, when using 'node->SetSelected(false);'

What I told above about the data roles was wrong. That part works correctly. I made a mistake. After I set the selection of the selection model, like in QmitkAbstractView::FireSelectedNodes(), I also set the selection through the selection provider that overwrote what I've just set.

But I think I found an actual bug in QmitkDataManagerView::NodeSelectionChanged(), here:

https://phabricator.mitk.org/source/mitk/browse/master/Plugins/org.mitk.gui.qt.datamanager/src/QmitkDataManagerView.cpp;423885b59a095ded50cf75c8a3d1601f9e5dba98$1116

When the data manager selection is set via BlueBerry, the selection of the internal selection model is set here:

https://phabricator.mitk.org/source/mitk/browse/master/Plugins/org.mitk.gui.qt.common/src/QmitkDataNodeSelectionProvider.cpp;423885b59a095ded50cf75c8a3d1601f9e5dba98$57

This will emit a signal that the data manager view connects to its NodeSelectionChanged() function.

This function is supposed to update the "selected" properties when the user clicks into the tree view. First, it sets every node unselected, then it goes through the "current selection" and sets the "currently selected" nodes selected.

The problem is that the "current selection" comes from the selection provider of the active view, while the signal came from the selection provider of the data manager. This is wrong. If you set the data manager selection from your (active) view, before you set the selection of your view itself, the selected properties will be updated incorrectly in the NodeSelectionChanged() function.

I found two solutions. First is to replace the GetCurrentSelection() call in NodeSelectionChange() so that the selection is taken from the selection provider of the data manager, and not the active part. The second, probably better, is to block the signals of the selection model in QmitkDataNodeSelectionProvider.

I will update the PR.

That's exactly the behaviour I discovered and reported earlier. However, as written above, when using 'node->SetSelected(false);' instead of 'node->SetBoolProperty("selected", false);' here
https://phabricator.mitk.org/source/mitk/browse/master/Plugins/org.mitk.gui.qt.datamanager/src/QmitkDataManagerView.cpp;423885b59a095ded50cf75c8a3d1601f9e5dba98$1112
the "current selection" is actually the correct, valid selection I excepted.

The only difference, as we already indentified is, that the later function call does modify the property list of the data node.

Still investigating.

Actually, there was no PR, so I opened one:

https://github.com/MITK/MITK/pull/204

Does this resolve your weird "selected" property setting issue?

I can also imagine that we see different behaviour because I am testing with 2015.05.2, and just rebased my patch on the master.

Hm.. Now the signal is not emitted, QmitkDataManagerView::NodeSelectionChanged() will not be called. That is ok, since it does nothing else than setting the selected property, which was now already done in the QmitkDataNodeSelectionProvider::SetSelection. However, since the signal is not emitted, the tree view is not updated, meaning the gray background does only change if the data manager view is updated on request (e.g. by moving the mouse onto the data manager view).

The weird "selected" property setting issue does not occur anymore, though (obviously, since the QmitkDataManagerView::NodeSelectionChanged() is not called anymore).
I suspect the modified property list triggers something. E.g. the re-rendering of the selected image (by the RenderingManager) which overwrites the current active selection or something like this.

I thought about that blocking the signals might cause such errors, but it did not occur here, so I thought it is fine. It may depend on the platform as well, and the qt version. I use Qt4 on Mac.

What if you remove the blocking but replace GetCurrentSelection in the data manager view?

Can you try this patch?

Do you have the Properties view open? I know that it is listening to the property list changes because the ReplaceProperty calls caused crash. I reported this not long ago and Stefan fixed it. It would be surprising, but maybe...

Yes, I have the properties view open. But closing it does not change the behavior.
The properties view uses the name of the currently selected node to label it's tab. When I click on my render window manager plugin and select an item, the label of the properties view is reset (no name in the tab title), which also shows that the current active selection is lost.

So, the render window manager plugin uses its own selection provider, right? If you set its selection and and right after you get the selection from it, do you get the back the same?

Yes, it uses the selection provider, created in QmitkAbstractView::SetSelectionProvider(). But I overrode the QmitkAbstractView::GetDataNodeSelectionModel to provide my own selection model, which is the Qt selection model of the table view, used in the render window manager.
The patch does not change the behavior.

If I use node->SetSelected(false); but then add node->GetPropertyList.Modified();, it does not work.

I would try this->GetSelectionProvider()->SetSelection(...) and right after this->GetSelectionProvider()->GetSelection() and check if the two are the same.

If I do this

Still I don't see the connection to the working version, where I changed the setting of the selected property of the node.

Here is what happens, if I synchronize the selection in the render window manager view with the selection in the data manager view:
If I use the node->SetBoolProperty(...) the selection is lost:

If I use the node->SetSelected(...) the selection is kept:

The only way I see to figure out the difference between the two ways of setting the "selected" property is to put a breakpoint at SetBoolProperty("selected") and to through all the listeners, step by step, systematically. Hopefully there are not so many.

I've looked again into this pull request and have a suggestion:

If I use your proposed solution

First is to replace the GetCurrentSelection() call in NodeSelectionChange() so that the selection is taken from the selection provider of the data manager, and not the active part.

then the NodeSelectionChanged()-function will be called, when changing the selected nodes from somewhere else (as always). However, due to the changed node retrieval, the correct nodes are now set 'selected'. In this case, wouldn't it be sufficient to change the 'select' property in the NodeSelectionChanged()-function so that the property must not be set in the QmitkDataNodeSelectionProvider::SetSelection()-function?

Currently this works in my situation, where I'm setting the data manager selection from my selection view via SetDataManagerSelection(selection).

However, this does not solve the problem I mentioned with the focus lost. I have one idea left. If this does not work, I'm waiting for your response to include your pull request (resp. partially) and close this for now. I have already opened another Task for my problem.