On Wed, 19 Nov 2025 15:56:56 GMT, Andy Goryachev <[email protected]> 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
