On Fri, 7 Oct 2022 12:45:04 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> `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 > >> `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 > > I know it is possible _theoretically_. My point is for a valid offset in the > document, this particular line must _never_ return `null`. If it does, the > element tree is invalid. As such, we should rather catch the case of bad bidi > element tree by throwing NPE rather than pretending nothing bad happened. > > `BidiRootElement` is a `BranchElement`: > https://github.com/openjdk/jdk/blob/096bca4a9c5e8ac2668dd965df92153ea1d80add/src/java.desktop/share/classes/javax/swing/text/AbstractDocument.java#L2700-L2713 > > And > [`BranchElement.getElementIndex`](https://github.com/openjdk/jdk/blob/096bca4a9c5e8ac2668dd965df92153ea1d80add/src/java.desktop/share/classes/javax/swing/text/AbstractDocument.java#L2416-L2474) > always returns a *valid* index. > > So, I am against adding the `null`-check. It is not necessary here. If there is only one character inserted/present in the document and then undo-ed, then there is no character/bidiElement so getElement() will return null and then getEndOffset() on that null element will return NPE, for which I have added the null check. ------------- PR: https://git.openjdk.org/jdk/pull/10446
