JDK-8102169 : Node Orientation - API issue: Should effective orientation be a property?
  • Type: Enhancement
  • Component: javafx
  • Sub-Component: controls
  • Affected Version: 8
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • Submitted: 2012-11-08
  • Updated: 2015-06-16
  • Resolved: 2013-01-24
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8
8Fixed
Related Reports
Blocks :  
JDK-8100124 - Create API for determining the layout direction of a component
Blocks :  
JDK-8100148 - Add RTL support to Text node
Blocks :  
JDK-8102057 - Add CSS support for the directionality pseudo-class :dir()
Blocks :  
JDK-8090340 - Add CSS support for attribute selector
Blocks :  
JDK-8125160 - Text orientation not always inherit
Relates :  
JDK-8100141 - Add RTL support to TextField / TextArea / PasswordField
Relates :  
JDK-8117367 - change in node orientation does not traverse the entire tree
Relates :  
JDK-8124933 - Use effectiveNodeOrientationProperty in Text
Relates :  
JDK-8087529 - [MIRROR] setting orientation in scene does not propagate to trim
Relates :  
JDK-8099289 - Support component orientation in common UI controls
Description
>>
>>>
>>> > Shouldn't effectiveNodeOrientation be a property?
>>>
>>> That's a possibility.  It would be a properly that changed when inherited orientation up the ancestor tree changed.  Do we have any other properties like this in FX?
>>
>> localToSceneTransform But I admit there is some extra logic needed for such properties that we don't want to add blindly for performance reasons. So it may be better to just rename the getter to simply effectiveNodeOrientation().
>>
>
> It might be that this needs to be a property after all.  The issue is that a child may have state that is sets based on effective orientation (say alignment of a text node) and this state needs to be kept up to date with effective orientation.  However, providing the method is defined correctly, there is nothing stopping it from becoming a property in future.  I understand the performance issue.  I will investigate further.

For a property we'd have effectiveNodeOrientationProperty() and getEffectiveNodeOrientation(). For a non-property we'd have something like effectiveNodeOrientation(). So I think we need to decide in the beginning..
Comments
getEffectiveNodeOrientation is a property now - verified on 8.0b88
06-05-2013
Changeset: 9d502f03ee58 Author: Lubomir Nerad <lubomir.nerad@oracle.com> Date: Thu Jan 24 10:16:38 2013 +0100 URL: http://jfxsrc.us.oracle.com/javafx/8.0/scrum/graphics/rt/rev/9d502f03ee58 Description: Fix for RT-26140: Effective node orientation as property
24-01-2013
@Steve: "there are most likely performance implications" Very good point. If I understand right, any Node that has its orientation set to INHERIT will need to add a listener to the parent's effective orientation (and remove / switch listeners when parents change). This way the only time the parent will "walk down the tree" to notify children that its effective orientation has changed is when a child needs to know that information, and only will visit those children that need to know it. Doing a general downward traversal would of course be really bad.
22-01-2013
With this property in place, it should be fairly straight-forward to implement a pseudo-class state solution for CSS support by overriding the property's invalidated method to call pseudoClassStateChanged. This could be tied into the :dir() pseudo-clas (RT-25288). RT-25287 is unlikely to happen for Lombard.
22-01-2013
This property is also needed for implementing CSS support, allowing developers to use different styles and resources depending on orientation. See RT-25287 and RT-25288.
22-01-2013
As I wrote in the API review before the code was pushed and this CR created, we have different naming conventions for properties and non-properties. So if it's going to be public, it has to be decided.
09-01-2013
The Text node now has a workaround to listen to changes to the transform, so we may be able to defer this decision for a later release.
04-01-2013
RT-26582 this case parent changes and Text does not get orientation because impl_transformsChanged is never called.
29-11-2012
effectiveOrientation also should fire when the node has a new parent (and orientation == inherit)
21-11-2012
Yes, any class that disables mirroring, as Text does, may need to be notified when its effective node orientation changes. Maybe a lighter weight alternative could be for Node to call a notification method instead of (or in addition to) applying the mirroring transform. See also RT-26386.
21-11-2012
Getting back to the topic: "Node Orientation - API issue: Should effective orientation be a property?" IMO, the answer is yes. In order to fix RT-26171 I need to know when the node effective orientation is changed so the text node knows when to update the text layout with the new orientation.
21-11-2012
>That should be doable. Couldn't I just set the nodeOrientation and/or textAlignment to be the same on all the following paragraph Text nodes? Yes.
13-11-2012
> How to we handle multi lines (multi paragraphs) ? > The first strong char in the text sets the base direction to all lines -or- we need to sniff each lines independently ? > Note that in Text that is not problem with the non-sniffing solution. Except that you will have the problem on control if you decide to sniff on top of text. That should be doable. Couldn't I just set the nodeOrientation and/or textAlignment to be the same on all the following paragraph Text nodes?
13-11-2012
>So an app could override the replaceText()/replaceSelection() methods in text controls and make sure the text always starts with a bidi control character saying it's LTR? >That sounds fairly easy. not really, it will need to fix arrow next / arrow previous so that the caret does not move before LTR mark, fix home, every api that takes or return a char offset will be off by one, etc >Why would it make the bidi sniffing harder? Since the base direction is not know till the text is set, the application will have to first analysis, then add the bidi controls , then set the text again. Or add extra bidi controls. If the base direction is know, it is a bit easier. Anyhow, this is not a big problem. Here another question (applies to sniffing and non-sniffing solution): How to we handle multi lines (multi paragraphs) ? The first strong char in the text sets the base direction to all lines -or- we need to sniff each lines independently ? Note that in Text that is not problem with the non-sniffing solution. Except that you will have the problem on control if you decide to sniff on top of text.
13-11-2012
So an app could override the replaceText()/replaceSelection() methods in text controls and make sure the text always starts with a bidi control character saying it's LTR? That sounds fairly easy. Why would it make the bidi sniffing harder?
13-11-2012
>Here is a case: I have a text field in a file dialog that contains a path. The BIDI algorithm should never be applied to it because it is a path. Can I get what I want (with or without sniffing)? Neither, for a path we need bidi segments or direct the app to use bidi control characters. Actually, sniffing would make the code harder in the case.
13-11-2012
That's fine for now.
13-11-2012
Here is a case: I have a text field in a file dialog that contains a path. The BIDI algorithm should never be applied to it because it is a path. Can I get what I want (with or without sniffing)?
13-11-2012
I see, in the impossibility to decided when to sniff I suggested to never sniff, while you (Leif) suggested to always sniff. What is going to be ? never sniff: good) easy to implement on Text, makes node orientation inheritance consistent to all nodes. bad) if the client wants sniffing it has to implement it on top of the text node. I suppose we can make that easier by offering a getBaseTextOrienation() api in Text. The implementation this getBaseTextOrienation() api would be pretty much the same as: Bidi bidi = new Bidi(getSkinnable().getText(), (no == LEFT_TO_RIGHT) ? Bidi.DIRECTION_DEFAULT_LEFT_TO_RIGHT : Bidi.DIRECTION_DEFAULT_RIGHT_TO_LEFT); if ((no == LEFT_TO_RIGHT) != bidi.baseIsLeftToRight()) { With the benefits that the Text node has already done this work (no need for the user to re-do the bidi analysis). Thoughts ? I think we have to go with the "never sniff" option. The problem with the "always sniff" options is that the user can't opt out it.
13-11-2012
I have successfully adapted TextFieldSkin to use java.text.bidi, so I don't need to rely on Text.getEffectiveNoOrientation(). TextField now aligns the text automatically when the user changes the text content between LTR and RTL. We may consider adding a style property to allow developers to override this. See patch-excerpt.txt for details.
13-11-2012
Pavel, it seems we are changing our position on this and overloading node orientation to force the BIDI algorithm. This was not the original thinking but node orientation has no meaning on a text node (ie. we never want the text to do anything other than draw from left to right, using the bidi algorithm or not, it should never mirror etc.). This observation has caused us to overload the idea of node orientation to indicate the BIDI algorithm on text nodes.
13-11-2012
[Sorry for the edits.] I'm working on adding this code in TextFieldSkin: private HPos getHAlignment() { HPos hPos = getSkinnable().getAlignment().getHpos(); if (getSkinnable().getEffectiveNodeOrientation() != textNode.getEffectiveNodeOrientation()) { if (hPos == HPos.LEFT) { return HPos.RIGHT; } else if (hPos == HPos.RIGHT) { return HPos.LEFT; } } return hPos; } Can I replace the condition with the following instead? It seems a bit heavy. NodeOrientation no = getSkinnable().getEffectiveNodeOrientation(); Bidi bidi = new Bidi(getSkinnable().getText(), (no == LEFT_TO_RIGHT) ? Bidi.DIRECTION_DEFAULT_LEFT_TO_RIGHT : Bidi.DIRECTION_DEFAULT_RIGHT_TO_LEFT); if ((no == LEFT_TO_RIGHT) != bidi.baseIsLeftToRight()) {
13-11-2012
However, as I mentioned a number of times, I'm happy to just get an API on Text to inquire about the strong (bidi) direction instead. The text input controls need this.
13-11-2012
I'm now totally confused. I thought we had agreed that Text should return a value for getEffectiveNodeOrientation() based mainly on sniffing. My suggestion was to only do that for now, and possibly implement looking at the nodeOrientation property as a separate issue when we can figure out if it has been set. No sniffing would lead to a very bad user experience.
13-11-2012
I agree. Let's get it working consistently without sniffing. I believe that will result in a usable system and provide programmers with control in circumstances where they know the data is LTR (for example, a text field that is a file path) and should not use the bidi algorithm.
13-11-2012
Okay, in that case I think we should always set DIRECTION_LEFT_TO_RIGHT / DIRECTION_RIGHT_TO_LEFT based on the text node effective orientation. That means that Text will not sniff the content to set its orientation. If necessary, it can add API for that in the future. Steve: revert what we just talked, apparently I can't decide when to sniff, therefore, no sniffing for now. Do we all agree with that ?
13-11-2012
> If the nodeOrientation in Text is directly set LTR - > DIRECTION_LEFT_TO_RIGHT > If the nodeOrientation in Text is directly set RTL - > DIRECTION_RIGHT_TO_LEFT Since it is not possible for Text to detect whether the nodeOrientation property has been set explicitly (note that INHERIT is also an acceptable explicit value), and this is a rare use case, I propose that we deal with it as a separate issue with lower priority.
13-11-2012
See java.text.Bidi DIRECTION_LEFT_TO_RIGHT DIRECTION_RIGHT_TO_LEFT DIRECTION_DEFAULT_LEFT_TO_RIGHT DIRECTION_DEFAULT_RIGHT_TO_LEFT That defines the bidi algorithm for the text node: If the nodeOrientation in Text is directly set LTR - > DIRECTION_LEFT_TO_RIGHT If the nodeOrientation in Text is directly set RTL - > DIRECTION_RIGHT_TO_LEFT If the nodeOrientation in Text is inherit from the parent, and it is LTR -> DIRECTION_DEFAULT_LEFT_TO_RIGHT If the nodeOrientation in Text is inherit from the parent, and it is RTL -> DIRECTION_DEFAULT_RIGHT_TO_LEFT If the base direction in the text, after the bidi algorithm is applied resolves to LTR: The node effective orientation is LRT, left aligned is visually on the left, and right is visually on the right. If it resolves to RTL: The node effective orientation is RTL, left aligned is visually on the right, and right is visually on the left. Text is mirrored always returns false (so that the -1 on x axis is not applied to it).
13-11-2012
I was probably confused by the API proposal saying "Node orientation is different from BIDI". So, how exactly should it work? Does setting nodeOrientation on text have any effect at all?
13-11-2012
RT-26171 explains why Text needs to decide its effective node orientation independently of the nodeOrientation property.
13-11-2012
Why does text need to override it? Isn't it always the value of the "nodeOrientation" property or the parent's value if the former is INHERIT?
13-11-2012
It's possible that the text controls don't actually need this to be an observable property in Text after all, because any changes to the Text node's properties are set from the control skin itself. In other words, for this use case, the Text node would only change its strong direction dynamically if it was triggered by a change from the text control. The method getEffectiveNodeOrientation() still needs to be made non-final so Text can override it. It might need a slightly different name if it's not a property. Maybe just effectiveNodeOrientation()? Compare to the naming of Region.getPrefWidth() vs Node.prefWidth() where the latter, which is not a property, is used to get the effective preferred width.
12-11-2012
I am still working on RT-24233, rewriting a lot of the code in the text controls' skin classes to handle RTL (for alignment, caret positioning, mouse and keyboard events, etc). The tricky part is when the Text's direction != the Control's orientation. I am not sure yet about how dependent this will be on the Text's strong direction, if at all. I should know more in a day or two.
12-11-2012
It would be nice if this was a property but there are most likely performance implications. It's like "enabled" and "visible" in that children are disabled and not visible when the parent property is changed. I believe that we don't have properties on FX for this. Do you know of any?
12-11-2012
Leif, please elaborate. The only case I can think of is to update the caret (and selection) location. But the way I thought it the text control will be fine as long as it listem to change the caret shape, which will be invalidate when the orientation changes. Is there any other cases ?
12-11-2012
This needs to be an observable property, at least in Text, so that the text input controls can react to a change in the strong direction based on user input.
12-11-2012
Hmm Do we really need this property ? Maybe we can use is[Automatically]Mirrored() to implement localToSceneTransform ? Overall I think RT-26215 is a better API, but I don't think we will have this feature any time soon.
12-11-2012
Raising prioirty to indicate that we cannot ship with this unresolved.
12-11-2012
See RT-26171 for a use case. If making this a property in Node is too heavyweight, then Text could have its own property instead to reflect the value for listeners. Are there other use cases? Would SceneBuilder neeed this?
10-11-2012