On Wed, 19 May 2021 17:04:12 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
>> 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. Thanks jonathan! Since I have documented the behavior of `isValid` and all the tests pass, I would assume my changes as of now is ready for this issue. For the further practice, I guess `isValid()` calls will be replaced by `!isEmpty()` then? But this revamp is out of the scope of this issue. ------------- PR: https://git.openjdk.java.net/jdk/pull/4066
