On Wed, 19 May 2021 17:01:08 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
>> 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. > > My general recollection is that this dates all the way back to the > introduction of `HtmlTree`. Prior to that, HTML was generated directly with > `PrintWriter.print` calls (!) and that code gave rise to some notable errors; > specifically, empty lists. A list was started, beginning with `<ul>` or > `<dl>`, items were added, and the list was closed. Except that sometimes no > items were added. Which was generally tolerated by browsers, but not by > checkers like `HTML Tidy` when we started using that. > > Roll the clock forward to the introduction of `HtmlTree`. For the most part, > this was done at that time by a mostly direct translation of `print` calls > into creating and adding `HtmlTree` nodes. The "empty list" problem was > solved by checking _ex post facto_ that nodes were valid before adding them > into an enclosing container. > > The conversion was a big enough job in itself that it was not reasonable to > restructure the code at that time to decide whether we should even be > creating the list in the first place. It would be better to check the > collection of items to be added up front, and not attempt to create an > invalid HTML list with no contents in the first place. > > I've not (yet) followed this thread in detail, but IMO the `isValid` > mechanism may be a wart of Technical Debt that it is time to remove. If > nothing else, the general improvements to the API over the past few years > should mean that we should always and only create valid nodes in the first > place, and that we should never create or need to check for invalid nodes. > That being said, such a change may be a Big Deal and should probably be > handled separately. Exercise for the reader: it may be an interesting exercise to temporarily instrument javadoc to see if we still create any invalid nodes in either `make docs` or in any test. ------------- PR: https://git.openjdk.java.net/jdk/pull/4066