The Property view steals PropertySheetPages from other views

[DieterMaiSysgo]

• I verified I can reproduce this issue against [latest Integration Build of Eclipse SDK]

I tested this with org.eclipse.ui.views_3.12.100.v20230821-1342.jar. This is a year old but the code that (I assume) causes the issue has not changed since then.

The Issue

So i have this view (VariablesView) that extends PageBookView. The pages that VariablesView hosts extend from TabbedPropertySheetPage which implements IPropertySheetPage.

Everything worked fine. Until one day, for some reason i had the Properties View open at the same time as my VariablesView. Suddenly the pages of the VariablesView were partially rendered in the Properties View. See screenshot

The Cause

Analyzing this issue i determent that the following code in PropertySheet.java seems to be the issue:

	@Override
	protected PageRec doCreatePage(IWorkbenchPart part) {
		// Get a custom property sheet page but not if the part is also a
		// PropertySheet. In this case the child property sheet would
		// accidentally reuse the parent's property sheet page.
		if(part instanceof PropertySheet) {
			return null;
		}
		IPropertySheetPage page = Adapters.adapt(part, IPropertySheetPage.class);
		if (page != null) {
			if (page instanceof IPageBookViewPage) {
				initPage((IPageBookViewPage) page);
			}
			page.createControl(getPageBook());
			return new PageRec(part, page);
		}

This is executed whenever the PropertiesView is open and a WorkbenchPart is select. In my case: The VariablesView can be adapted to a IPropertySheetPage because PageBockView adapts the current page which is an IPropertySheetPage. As a consequent createControl() is invoked with a new parent composite on a page that was already created. Any new child widgets will be added to the Properties view.

The Workaround

For me, this is not much of an issue since i can override getAdapter() method so that my VariablesView is never adapted to an IPropertySheetPage.

Conclusion

I ask myself, where exactly this went wrong. Maybe it is wrong to use IPropertySheetPage or a TabbedPropertySheetPage in a view other then the Properties View.
On the other hand invoking createControl() on a IPage that came from an Adapter is also very risky since any client would expect that createControl() is invoked not more then once.

Maybe this could be fixed by checking if the IPropertySheetPage already is created. I don't see any reason why a IPropertySheetPage should be created multiple times

		IPropertySheetPage page = Adapters.adapt(part, IPropertySheetPage.class);
		if (page != null && page.getControl() == null) { // <-- Fix
			if (page instanceof IPageBookViewPage) {
				initPage((IPageBookViewPage) page);
			}
			page.createControl(getPageBook());
			return new PageRec(part, page);

Let me know what you think. Is this something that needs to be fixed or is working as intended?

Community

• I understand reporting an issue to this OSS project does not mandate anyone to fix it. Other contributors may consider the issue, or not, at their own convenience. The most efficient way to get it fixed is that I fix it myself and contribute it back as a good quality patch to the project.

[merks]

The documentation of IPropertySheetPage says:

Interface for a property sheet page that appears in a property sheet view.

So given you are using it in another context for another purpose than to be a page in a property sheet view it seems reasonable you'd have to disable that behavior. That being said, the guard doesn't look harmful because the created page is managed by mapPartToRec which avoids the createControl method being called more than once.