RFR: 8371067: RichTextArea: requestLayout by inline node doesn't reach VFlow

Marius Hanl mhanl at openjdk.org
Thu Nov 20 08:33:16 UTC 2025


On Wed, 19 Nov 2025 15:56:56 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> modules/jfx.incubator.richtext/src/main/java/com/sun/jfx/incubator/scene/control/richtext/util/RichUtils.java line 751:
>> 
>>> 749:     // with the author's permission
>>> 750:     /** returns a parent of the specified type, or null.  if node is an instance of the specified class, returns node */
>>> 751:     public static <T> T getAncestorOfClass(Class<T> c, Node node) {
>> 
>> I would really recommend to not do anything like that. Normally, the layouting system, especially when requesting a layout should work and bubble up correctly. I wonder why this is needed. And if we found a special case, if we can solve it better.
>> 
>> Background: In the past projects, I often saw code like that and it turned out that this was never needed. It is often less readable and hurts the performance a bit. 
>> We also improve coupling between components, which I would not recommend as well. Especially since subclasses can change a lot and it would be nice if everything still work out of the box.
>
> Thank you @Maran23 for looking into this PR!
> 
> My experience is exactly opposite - I do use it often.  The `Node` (or `JComponent`) hierarchy is a hierarchy, explicitly retaining the pointers to the each member's parent.
> 
> A specific member can be a child of a certain Parent, direct or otherwise, by design, and this method allows to get to that parent easily.
> 
> Let me ask you this - what is the alternative?  Maintain a duplicate pointer?
> 
> Also, keep in mind this is not public API, it's a utility.

What I mean is:
Everything is part of the `RichTextLabel`. Requesting a layout from a node inside should request a layout for each parent, also `RichTextLabel`. So you should normally know that on the `RichTextLabel` level, which also manages the `VFlow`. So it can do the corresponding actions, just by the node requesting the layout.

If we take a look at other more complex Controls, they do the following:
- `VirtualFlow.requestCellLayout` -> sets a flag
- `layout` is requested, and due to the flag, we know what to do

Maybe something that could be done here as well? Could also be done by `Properties` perhaps.

> A specific member can be a child of a certain Parent, direct or otherwise, by design, and this method allows to get to that parent easily.
> ...
> Also, keep in mind this is not public API, it's a utility.

Yes, we can always get the parent hierarchy. But that does not mean we should. 
Making assumptions about the hierarchy will make subclasses and customizations (e.g. in the Skin) worse. If we extend `RichTextLabel` and use another Node then `VFlow`, then we can expect `TextCell`s not to work anymore?

In JavaFX, as you can also see in the codebase and other controls, retrieving an ancestor somewhere in the scene graph is pretty much never done or needed. 
I did not have a look on this particular issue, but what I want to suggest is to take another look at the problem and how to solve it. So we don't need to rely on finding a specific node that might be somewhere in the scene graph.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1975#discussion_r2544829563


More information about the openjfx-dev mailing list