On Mon, 3 Oct 2022 11:20:17 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:

>> Prasanta Sadhukhan has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Test fix
>
> src/java.desktop/share/classes/javax/swing/text/AbstractDocument.java line 
> 958:
> 
>> 956:                 int index = bidiRoot.getElementIndex(p0);
>> 957:                 Element bidiElem = bidiRoot.getElement(index);
>> 958:                 if (bidiElem != null && bidiElem.getEndOffset() >= p1) {
> 
> Is it possible that `bidiElem` is `null`? It should never be. If it is, it is 
> a bug in the code and throwing NPE seems good — it will be the indication of 
> the bug.
> 
> Since the NPE has never been thrown from this code, I'd rather leave it 
> unchanged here.

`getElement()` can return null 
[here](https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/javax/swing/text/AbstractDocument.java#L2619)
 and 
[here](https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/javax/swing/text/AbstractDocument.java#L2398)
so it should not be enitrely impossible not to get null, so it should be a ok

> test/jdk/javax/swing/text/AbstractDocument/TestUndoError.java line 39:
> 
>> 37: import javax.swing.undo.UndoManager;
>> 38: 
>> 39: public class TestUndoError {
> 
> `TestUndoInsertArabicText`? It's more specific this way.

ok

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

PR: https://git.openjdk.org/jdk/pull/10446

Reply via email to