flow: Node.removeFromParent does not set parent of child node to null when called from onDetach

Description of the bug

While debugging another issue for Vaadin CDI (see PR vaadin/cdi#344 and issue vaadin/cdi#345). I found that when removing a node from its parent will not remove the parent from the child. So consider a Component c with an Element e = c.getElement() and a parent Element p = e.getParent(). Now consider I would remove the element e from its parent p by e.removeFromParent() now the parent p will not contain e anymore, i.e. the following is true

p.getChildren().noneMatch(n -> n == e)

However, I can still do e.getParent(), which will result in p, i.e. the following is false:

e.getParent() == null

~Minimal reproducible example~ (new example in later comments)

removeFromParentAndCheck(HasElement he) {
  Element e = he.getElement();
  Element p = e.getParent();
  e.removeFromParent();
  assert p.getChildren().noneMatch(n -> n == e) : "element e should be removed";
  assert e.getParent() == null : "e should now have no parent";
}

Expected behavior

I basically would expect the child to have no parent

Actual behavior

I observe that the child still has his old parent

Versions:

- Vaadin / Flow version: 14.2.0
- Java version: JDK 11.0.7
- OS version: Windows 10

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 16 (6 by maintainers)

Most upvoted comments

The test is failing because of IAE:

java.lang.IllegalArgumentException: Trying to detach an element from parent that does not have it.
	at com.vaadin.flow.dom.impl.AbstractNodeStateProvider.removeChild(AbstractNodeStateProvider.java:123)
	at com.vaadin.flow.dom.Node.removeChild(Node.java:376)
	at com.vaadin.flow.dom.Element.removeFromParent(Element.java:514)

on the line : otherComponent.getElement().removeFromParent().

And this is absolutely expected : onDetach of the parent removes the otherElement from parent : otherComponent.getElement().removeFromParent( ).

It is when one removes a parent in an overridden onDetach method. There is no overridden onDetach method. The default onDetach method doesn’t do anything: it’s no-op. So there is no any need to call super.onDetach( detachEvent ) and effectively it’s the same as adding a detach listener. There is no any need to use components in this test : as I said onDetach doesn’t produce any side effects because super impl doesn’t do anything. Same can be done using just Element API.

I don’t see an issue here. Closing the ticket since no steps which allows to reproduce the ticket are provided. Apparently the issue is in your code.

denis-anisimov on Jun 26, 2020