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

onAfterAttach being run when widget not attached #9960

Closed
colin-grant-work opened this issue Aug 26, 2021 · 1 comment · Fixed by #9967
Closed

onAfterAttach being run when widget not attached #9960

colin-grant-work opened this issue Aug 26, 2021 · 1 comment · Fixed by #9967
Labels
layout restoration issues related to layout restoration shell issues related to the core shell

Comments

@colin-grant-work
Copy link
Contributor

colin-grant-work commented Aug 26, 2021

Bug Description:

Recently I have been seeing a number of messages like this in my console:

image

Many have to do with code introduced in #9935

Widget.attach(this.toolbar, this.header);

But at least one (the one in the screenshot above) points here:

Widget.attach(this.searchBox, this.node.parentElement!);

Which predates #9935 and probably wasn't affected by it (🤞).

What's happening in that somehow, when the onAfterAttach handlers are run, the widget's node is not contained in document.body

image

And that triggers this error in Phosphor:

https://github.com/phosphorjs/phosphor/blob/9f5e11025b62d2c4a6fb59e2681ae1ed323dcde4/packages/widgets/src/widget.ts#L1003-L1005

Adding a simple check like if (this.isAttached) doesn't seem to fix the problem, and it's very strange that an onAfterAttach handler is running but the widget claims that it is not attached.

Steps to Reproduce:

  1. Start the application (browser mode)
  2. Check the console
  3. Observe a number of Host is not attached errors logged.

Additional Information

  • Operating System: RHEL7
  • Theia Version: 16f3309
@colin-grant-work colin-grant-work changed the title onAfterAttach being run _before_ attachment onAfterAttach being run when widget not attached Aug 26, 2021
@colin-grant-work
Copy link
Contributor Author

colin-grant-work commented Aug 26, 2021

There are a number of places where we run code like this:

MessageLoop.sendMessage(widget, Widget.Msg.BeforeDetach);
this.parent!.node.removeChild(widget.node);
MessageLoop.sendMessage(widget, Widget.Msg.AfterDetach);
MessageLoop.sendMessage(widget, Widget.Msg.BeforeAttach);
this.parent!.node.insertBefore(widget.node, this.handles[toIndex]);
MessageLoop.sendMessage(widget, Widget.Msg.AfterAttach);

Which is basically the same as Widget.detach and Widget.attach, except it doesn't check for whether the widget can be attached / detached as far as Phosphor is concerned. It seems like that probably points to areas where we're doing things that our widgets don't expect to be done.

@colin-grant-work colin-grant-work added layout restoration issues related to layout restoration shell issues related to the core shell labels Aug 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
layout restoration issues related to layout restoration shell issues related to the core shell
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant