On Wed, 19 May 2021 12:08:27 GMT, Pavel Rappo <pra...@openjdk.org> wrote:
>> I was initially unsure about this, too. But take a look at
>> `HtmlTree.add(Content)` which handles `ContentBuilder` arguments in the
>> following way:
>>
>> if (content instanceof ContentBuilder) {
>> ((ContentBuilder) content).contents.forEach(this::add);
>> }
>>
>> Thus, if a ContentBuilder contains any valid Content objects, they will be
>> added, while the invalid ones will be silently dropped.
>
> Something is wrong, but I cannot quite put my finger on it. I think this PR
> merely highlights a pre-existing issue that should be investigated separately.
>
> I'm surprised by the concept of validity for the **generated** content. I'm
> also surprised by the fact that a part of the content can be silently omitted
> depending on whether or not that part is valid. This suggests that javadoc
> uses instances of HtmlTree as its data model which it shouldn't do.
This special path for `ContentBuilder` was added in
[JDK-8026370](https://bugs.openjdk.java.net/browse/JDK-8026370), commit
99e02c21cdb25bd113dd751f63f0c1fd27f07b57, which, surprisingly, is like the
opposite of this missing empty tag issue; the `isValid` was not touched since
its addition from long ago in
[JDK-6851834](https://bugs.openjdk.java.net/browse/JDK-6851834). ContentBuilder
itself was introduced in
[JDK-8011642](https://bugs.openjdk.java.net/browse/JDK-8011642) or commit
f961eafe351c24536e539819e9efb14272701195.
It is not quite clear to me the original intentions of `isValid` from over one
decade ago. One of the clue may be that when `ContentBuilder` was added,
jonathan maybe did not have `isValid` handling in mind? And we are now in a
somewhat weird shape.
-------------
PR: https://git.openjdk.java.net/jdk/pull/4066
