eclipse-platform / eclipse.platform.ui

Eclipse Platform
https://projects.eclipse.org/projects/eclipse.platform
Eclipse Public License 2.0
81 stars 188 forks source link

CompositeInformationControl replaces the original shell of its AbstractInformationControls #2242

Open markdomeng opened 2 months ago

markdomeng commented 2 months ago

at CompositeInformationControl : 111 (control.setParent(parent)) effectively replacing the original fShell of the AbstractInformationControl

this causes unexpected behaviors to its consumers. Some consumers I found while working on some LSP project (we are using GenericEditor):

(I am not able to reproduce it in a regular eclipse, everything mentioned above were only observed in our project)

Tested under this environment:

Community

markdomeng commented 2 months ago

proposed fix: https://github.com/markdomeng/eclipse.platform.ui/commit/dcd65aee08fcba8123cba9c2325c0d63b7bf1698

but this may cause issues to child classes relying on the getShell() to do something to its child controls, because it will now return a shell that contains controls other than the one it created .

example scenario: if CompositeInformationControl creates MarkerInformationControl and BrowserInformationControl, getShell() will now contain all controls created by both MarkerInformationControl and BrowserInformationControl

so in order to fix this, child classes of AbstractInformationControls should not rely on getShell() to access its child controls. I propose we also provide accessors to fContentComposite, and fStatusComposite, as an AbstractInformationControl will always have these 2

sample use case: in MarkerInformationControl : computeSizeHint() instead of getShell().getSize() we could return the size of the content + the size of the status composite as the actual size