<Swing Dev> RFR: 4710675: JTextArea.setComponentOrientation does not work with correct timing

[Prasanta Sadhukhan-2]

It is seen JTextArea.setComponentOrientation(ComponentOrientation.RIGHT_TO_LEFT) orientation is not honoured if it is called after setText() and remain at LTR orientation. It changes the orientation only if some more text is typed additionally to existing text.
This behaviour is different from JTextField where the RTL orientation is honoured from start.
Proposed fix is to check for ComponentOrientation propertyChange event and set i18n property if it is not set, so that orientation is honoured as soon as setComponentOrientation() is called.
Checked for all L&Fs in all supported platforms.

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

Commit messages:
 - 4710675: JTextArea.setComponentOrientation does not work with correct timing

Changes: https://git.openjdk.java.net/jdk/pull/2673/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2673&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-4710675
  Stats: 123 lines in 2 files changed: 123 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2673.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2673/head:pull/2673

PR: https://git.openjdk.java.net/jdk/pull/2673

[Alexander Zvegintsev-2]

On Mon, 22 Feb 2021 09:45:31 GMT, Prasanta Sadhukhan <[hidden email]> wrote:

> It is seen JTextArea.setComponentOrientation(ComponentOrientation.RIGHT_TO_LEFT) orientation is not honoured if it is called after setText() and remain at LTR orientation. It changes the orientation only if some more text is typed additionally to existing text.
> This behaviour is different from JTextField where the RTL orientation is honoured from start.
> Proposed fix is to check for ComponentOrientation propertyChange event and set i18n property if it is not set, so that orientation is honoured as soon as setComponentOrientation() is called.
> Checked for all L&Fs in all supported platforms.

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTextUI.java line 1908:

> 1906:             if ("focusAccelerator" == propertyName) {
> 1907:                 updateFocusAcceleratorBinding(true);
> 1908:             } else if ("componentOrientation" == propertyName) {

It is not a part of the fix, but shouldn't we use `equals()` instead of `==` for String comparison?

Please also update the copyright year.

test/jdk/javax/swing/JTextArea/JTextAreaOrientationTest.java line 48:

> 46:     static Rectangle bounds;
> 47:
> 48:     public static boolean compareBufferedImages(BufferedImage bufferedImage0, BufferedImage bufferedImage1) {

We could probably reuse `Util.compareBufferedImages()` from `regtesthelpers` here.

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

PR: https://git.openjdk.java.net/jdk/pull/2673

[Prasanta Sadhukhan-2]

On Mon, 22 Feb 2021 12:42:20 GMT, Alexander Zvegintsev <[hidden email]> wrote:

>> It is seen JTextArea.setComponentOrientation(ComponentOrientation.RIGHT_TO_LEFT) orientation is not honoured if it is called after setText() and remain at LTR orientation. It changes the orientation only if some more text is typed additionally to existing text.
>> This behaviour is different from JTextField where the RTL orientation is honoured from start.
>> Proposed fix is to check for ComponentOrientation propertyChange event and set i18n property if it is not set, so that orientation is honoured as soon as setComponentOrientation() is called.
>> Checked for all L&Fs in all supported platforms.
>
> test/jdk/javax/swing/JTextArea/JTextAreaOrientationTest.java line 48:
>
>> 46:     static Rectangle bounds;
>> 47:
>> 48:     public static boolean compareBufferedImages(BufferedImage bufferedImage0, BufferedImage bufferedImage1) {
>
> We could probably reuse `Util.compareBufferedImages()` from `regtesthelpers` here.

I intentionally didn't use Util library so I could use it as standalone test.

> src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTextUI.java line 1908:
>
>> 1906:             if ("focusAccelerator" == propertyName) {
>> 1907:                 updateFocusAcceleratorBinding(true);
>> 1908:             } else if ("componentOrientation" == propertyName) {
>
> It is not a part of the fix, but shouldn't we use `equals()` instead of `==` for String comparison?
>
> Please also update the copyright year.

I dont see the need. Also, it's not mandatory to change the copyright year which will be taken care of by script at end of release.

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

PR: https://git.openjdk.java.net/jdk/pull/2673

[Sergey Bylokhov-2]

On Mon, 22 Feb 2021 15:36:20 GMT, Prasanta Sadhukhan <[hidden email]> wrote:

>> src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTextUI.java line 1908:
>>
>>> 1906:             if ("focusAccelerator" == propertyName) {
>>> 1907:                 updateFocusAcceleratorBinding(true);
>>> 1908:             } else if ("componentOrientation" == propertyName) {
>>
>> It is not a part of the fix, but shouldn't we use `equals()` instead of `==` for String comparison?
>>
>> Please also update the copyright year.
>
> I dont see the need. Also, it's not mandatory to change the copyright year which will be taken care of by script at end of release.

I suggest changing the copyright year, did you remember then someone run any scripts for that?

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

PR: https://git.openjdk.java.net/jdk/pull/2673

[Sergey Bylokhov-2]

On Mon, 22 Feb 2021 09:45:31 GMT, Prasanta Sadhukhan <[hidden email]> wrote:

> It is seen JTextArea.setComponentOrientation(ComponentOrientation.RIGHT_TO_LEFT) orientation is not honoured if it is called after setText() and remain at LTR orientation. It changes the orientation only if some more text is typed additionally to existing text.
> This behaviour is different from JTextField where the RTL orientation is honoured from start.
> Proposed fix is to check for ComponentOrientation propertyChange event and set i18n property if it is not set, so that orientation is honoured as soon as setComponentOrientation() is called.
> Checked for all L&Fs in all supported platforms.

test/jdk/javax/swing/JTextArea/JTextAreaOrientationTest.java line 112:

> 110:             bounds = frame.getBounds();
> 111:         });
> 112:         BufferedImage img = robot.createScreenCapture(bounds);

Probably you do not need a robot just to see the difference in the rendering? Just draw to the buffered image and compare.

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

PR: https://git.openjdk.java.net/jdk/pull/2673

[Prasanta Sadhukhan-2]

> It is seen JTextArea.setComponentOrientation(ComponentOrientation.RIGHT_TO_LEFT) orientation is not honoured if it is called after setText() and remain at LTR orientation. It changes the orientation only if some more text is typed additionally to existing text.
> This behaviour is different from JTextField where the RTL orientation is honoured from start.
> Proposed fix is to check for ComponentOrientation propertyChange event and set i18n property if it is not set, so that orientation is honoured as soon as setComponentOrientation() is called.
> Checked for all L&Fs in all supported platforms.

Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:

  Remove robot reliance

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2673/files
  - new: https://git.openjdk.java.net/jdk/pull/2673/files/09052724..efec94fc

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2673&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2673&range=00-01

  Stats: 13 lines in 2 files changed: 6 ins; 3 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2673.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2673/head:pull/2673

PR: https://git.openjdk.java.net/jdk/pull/2673

[Sergey Bylokhov-2]

On Thu, 25 Feb 2021 04:37:01 GMT, Prasanta Sadhukhan <[hidden email]> wrote:

>> It is seen JTextArea.setComponentOrientation(ComponentOrientation.RIGHT_TO_LEFT) orientation is not honoured if it is called after setText() and remain at LTR orientation. It changes the orientation only if some more text is typed additionally to existing text.
>> This behaviour is different from JTextField where the RTL orientation is honoured from start.
>> Proposed fix is to check for ComponentOrientation propertyChange event and set i18n property if it is not set, so that orientation is honoured as soon as setComponentOrientation() is called.
>> Checked for all L&Fs in all supported platforms.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
>
>   Remove robot reliance

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTextUI.java line 1914:

> 1912:                 final String I18NProperty = "i18n";
> 1913:                 if (ComponentOrientation.RIGHT_TO_LEFT == newValue
> 1914:                     && ! Boolean.TRUE.equals(document.getProperty(I18NProperty))) {

Please add a comment similar to the text in the AbstractDocument.handleInsertString, otherwise, it is unclear why we need to enable "complex layout".

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

PR: https://git.openjdk.java.net/jdk/pull/2673

[Prasanta Sadhukhan-2]

> It is seen JTextArea.setComponentOrientation(ComponentOrientation.RIGHT_TO_LEFT) orientation is not honoured if it is called after setText() and remain at LTR orientation. It changes the orientation only if some more text is typed additionally to existing text.
> This behaviour is different from JTextField where the RTL orientation is honoured from start.
> Proposed fix is to check for ComponentOrientation propertyChange event and set i18n property if it is not set, so that orientation is honoured as soon as setComponentOrientation() is called.
> Checked for all L&Fs in all supported platforms.

Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:

  Add comment

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2673/files
  - new: https://git.openjdk.java.net/jdk/pull/2673/files/efec94fc..54955c85

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2673&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2673&range=01-02

  Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2673.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2673/head:pull/2673

PR: https://git.openjdk.java.net/jdk/pull/2673

[Sergey Bylokhov-2]

On Fri, 26 Feb 2021 05:32:06 GMT, Prasanta Sadhukhan <[hidden email]> wrote:

>> It is seen JTextArea.setComponentOrientation(ComponentOrientation.RIGHT_TO_LEFT) orientation is not honoured if it is called after setText() and remain at LTR orientation. It changes the orientation only if some more text is typed additionally to existing text.
>> This behaviour is different from JTextField where the RTL orientation is honoured from start.
>> Proposed fix is to check for ComponentOrientation propertyChange event and set i18n property if it is not set, so that orientation is honoured as soon as setComponentOrientation() is called.
>> Checked for all L&Fs in all supported platforms.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
>
>   Add comment

Marked as reviewed by serb (Reviewer).

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

PR: https://git.openjdk.java.net/jdk/pull/2673

[Pankaj Bansal-2]

On Fri, 26 Feb 2021 05:32:06 GMT, Prasanta Sadhukhan <[hidden email]> wrote:

>> It is seen JTextArea.setComponentOrientation(ComponentOrientation.RIGHT_TO_LEFT) orientation is not honoured if it is called after setText() and remain at LTR orientation. It changes the orientation only if some more text is typed additionally to existing text.
>> This behaviour is different from JTextField where the RTL orientation is honoured from start.
>> Proposed fix is to check for ComponentOrientation propertyChange event and set i18n property if it is not set, so that orientation is honoured as soon as setComponentOrientation() is called.
>> Checked for all L&Fs in all supported platforms.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
>
>   Add comment

Marked as reviewed by pbansal (Reviewer).

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

PR: https://git.openjdk.java.net/jdk/pull/2673

[Prasanta Sadhukhan-2]

On Mon, 22 Feb 2021 09:45:31 GMT, Prasanta Sadhukhan <[hidden email]> wrote:

> It is seen JTextArea.setComponentOrientation(ComponentOrientation.RIGHT_TO_LEFT) orientation is not honoured if it is called after setText() and remain at LTR orientation. It changes the orientation only if some more text is typed additionally to existing text.
> This behaviour is different from JTextField where the RTL orientation is honoured from start.
> Proposed fix is to check for ComponentOrientation propertyChange event and set i18n property if it is not set, so that orientation is honoured as soon as setComponentOrientation() is called.
> Checked for all L&Fs in all supported platforms.

This pull request has now been integrated.

Changeset: bcca1006
Author:    Prasanta Sadhukhan <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/bcca1006
Stats:     129 lines in 2 files changed: 128 ins; 0 del; 1 mod

4710675: JTextArea.setComponentOrientation does not work with correct timing

Reviewed-by: serb, pbansal

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

PR: https://git.openjdk.java.net/jdk/pull/2673