Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CompositeInformationControl replaces the original shell of its AbstractInformationControls #2242

Open
1 task done
markdomeng opened this issue Sep 5, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@markdomeng
Copy link

markdomeng commented Sep 5, 2024

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):

  • MarkerInformationControl

    • at line 163, computeSizeHint() : this returns a very small Point(2,2) because the child controls are stolen. (Only if it is created by CompositeInformationControlCreator at line 123 as a CompositeInformationControl)

    • at line 149, setInput() : the shell is not getting disposed after applying the quickfix

  • BrowserInformationControl

    • at line 225, createContent() : I only observed this when using dark mode, but the background color of the BrowserInformationControl is different from the MarkerInformationControl when they are created as a CompositeInformationControl. I suspect this is the cause

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

Tested under this environment:

  • OS & version: Windows 11
  • Eclipse IDE/Platform version (as shown in Help > About):
    • Version: 2024-06 (4.32)
    • Build Id: I20240601-0610

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.
@markdomeng markdomeng added the bug Something isn't working label Sep 5, 2024
markdomeng added a commit to markdomeng/eclipse.platform.ui that referenced this issue Sep 5, 2024
…ite is

in some cases, the original shell is being replaced by the child classes

a good example is in CompositeInformationControl, in createContent() where it collects all the controls of the AbstractInformationControl and sets a different parent to it

see issue: [eclipse-platform#2242](eclipse-platform#2242)
@markdomeng
Copy link
Author

proposed fix: markdomeng@dcd65ae

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant