RFR: 8049347: HTMLDocument throws NPE for Block Tag

Thu Jun 15 07:46:59 UTC 2023

On Thu, 15 Jun 2023 03:50:50 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> src/java.desktop/share/classes/javax/swing/text/html/HTMLDocument.java line 550:
>> 
>>> 548:         if (t.isBlock()) {
>>> 549:             // TBD
>>> 550:             return null;
>> 
>> This doesn't look right to me. The comment suggests the implementation of something like `BlockIterator` is missing.
>> 
>> I would expect that iterator for `HTML.Tag.HEAD` returns all elements inside `<head>` element. Yet the comment for the `next` method
>> 
>> https://github.com/openjdk/jdk/blob/931625a9304ec2761ca9035d69fd33f6beadb124/src/java.desktop/share/classes/javax/swing/text/html/HTMLDocument.java#L1993-L1997
>> 
>> suggests, the iterator goes over all instances of the specified tag in the document.
>> 
>> What does the iterator return if you pass it `HTML.Tag.HEAD` or `HTML.Tag.P`?
>
> It returns a LeafIterator object with HEAD or P tag respectively

Obviously. :)

Does it do what the `getIterator` method promise to do? It doesn't because it iterates over `LeafElement`-s only:

https://github.com/openjdk/jdk/blob/931625a9304ec2761ca9035d69fd33f6beadb124/src/java.desktop/share/classes/javax/swing/text/html/HTMLDocument.java#L1993-L1998

So, the `next` method moves the iterator to the next leaf element:

https://github.com/openjdk/jdk/blob/931625a9304ec2761ca9035d69fd33f6beadb124/src/java.desktop/share/classes/javax/swing/text/html/HTMLDocument.java#L2037-L2044

Note that it uses `isLeaf` to stop the generic `ElementIterator`.

---

It looks as if `HTMLDocument.getIterator` is public as an implementation detail: it's used to iterate over `HTML.Tag.A` to scroll the text component to the anchor and to determine whether the document is a frame set or not.

Removing the `if` condition removes the NPE but the method doesn't support Block Elements.

Should we update the javadoc to state the method shouldn't be used by apps? And to throw `UnsupportedOperationException` if a block tag is passed instead of returning null?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14466#discussion_r1230580722