On Tue, 11 Oct 2022 05:40:40 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org>
wrote:
>> javax.swing.text.AbstractDocument$BranchElement.replace(...) method throws
>> an `ArrayIndexOutOfBoundsException: arraycopy: length -1 is negative` when
>> using an UndoManager on the default document of a JTextArea and you try to
>> undo the insertion of a LEFT-TO-RIGHT language (e.g. Arabic) that is
>> immediately followed by setting the component orientation on the JTextArea.
>>
>> This is because System.arrayCopy() is called with -ve length because of the
>> calculation done in AbstractDocment.replace where `src` is of 2bytes because
>> of unicode text causing `nmove` to become -ve if `nchildren` is 1 (an
>> unicode character is inserted)
>>
>> System.arrayCopy throws `IndexOutOfBoundsException if:
>>
>> The srcPos argument is negative.
>> The destPos argument is negative.
>> The length argument is negative
>>
>>
>> so the fix is made to make nmove, src, dest +ve
>> Also, Element.getElement() can return null which is not checked, causing
>> NPE, if deletion of text is done which results in no element, which is also
>> fixed in the PR
>>
>> All jtreg testsuite tests are run without any regression.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Test update
This does **corrupt the document structure**.
The negative indexes are the sign that the undoable edit contradicts to the
current document structure and cannot be applied therefore. Ignoring that is a
call for trouble in another place.
What happens in the provided test case?
The document before the Arabic character is inserted:
--- empty ---
<paragraph>
<content>
[0,1][
]
<bidi root>
<bidi level
bidiLevel=0
>
[0,1][
]
This is the document dump obtained via
[`AbstractDocument.dump`](https://docs.oracle.com/en/java/javase/17/docs/api/java.desktop/javax/swing/text/AbstractDocument.html#dump(java.io.PrintStream)).
After the Arabic character is inserted:
--- one char ---
<paragraph>
<content>
[0,2][ر
]
<bidi root>
<bidi level
bidiLevel=1
>
[0,1][ر]
<bidi level
bidiLevel=0
>
[1,2][
]
Now the text component orientation is changed to RTL:
--- orientation changed ---
<paragraph>
<content>
[0,2][ر
]
<bidi root>
<bidi level
bidiLevel=1
>
[0,2][ر
]
The most important change is that bidi-root now contains only one element with
level 1 which contains the entire document content whereas before the
orientation was changed it had contained two elements with level 1 and 0.
The undoable edit stores the data to restore the bidi-root from two leaf
elements to one leaf. Yet now the bidi-root has only one leaf element. That's
why the indexes in `replace` are negative: the undoable edit cannot be applied
correctly.
Let's continue, as the indexes are positive now and the undo can be performed.
--- undone ---
<paragraph>
<content>
[0,1][
]
<bidi root>
This results in bidi-root which has no elements. This is _**not** a valid
document_ any more. I [mentioned
above](https://github.com/openjdk/jdk/pull/10446#discussion_r985670722) that
`bidiElem` in `isLeftToRight` must never be `null`. It *is* `null` because
converting negative indexes to positive in `replace` breaks the structure.
The proposed fix must not be integrated.
Is there a solution? I'm afraid there's none.
The best approach would be to invalidate all the undo information in the
`UndoManager`. *Nothing can be undone* after the orientation of a text
component is changed. This is true even if the edit doesn't throw an exception.
For example, if the document contains the following text: "start
<Arabic-char> end". The insertion of an Arabic character is the last
operation, it will be undone when `undo` is called. The component orientation
was changed to RTL after inserting that Arabic character, which changes the
levels in bidi-root from 0-1-0 to 2-1-2. Calling `undo` undoes the changes and
restores the levels which had been before the orientation was changed, that is
to 0 instead of the expected 2. This results in wrong rendering of
bidirectional text.
I came up with a test case which throws `ArrayIndexOutOfBounds` before the fix
is applied. With the fix applied, it throws an exception on EDT:
Exception in thread "AWT-EventQueue-0" java.lang.IllegalArgumentException:
offsetLimit must be after current position
at
java.desktop/java.awt.font.LineBreakMeasurer.nextOffset(LineBreakMeasurer.java:356)
at
java.desktop/javax.swing.text.TextLayoutStrategy.getLimitingOffset(TextLayoutStrategy.java:286)
at
java.desktop/javax.swing.text.TextLayoutStrategy.createView(TextLayoutStrategy.java:194)
at ...
What can we recommend? Set the component orientation *before* editing text and
capturing undoable edits.
-------------
PR: https://git.openjdk.org/jdk/pull/10446
