> Please review a change to straighten out the mechanism to avoid generating > empty HTML elements in javadoc. > > Currently this is implemented using the `Content.isValid()` method in > `HtmlTree.add(Content)` to check whether the the content argument should be > added or dropped. This seems wrong since we don't really want to check for > validity but non-emptiness (we want to drop empty elements, not print a > warning or error). > > So the first decision was to get rid of the `Content.isValid()` method and > use `Content.isEmpty()` instead. I considered keeping `isValid()` and using > it to actually do some validation checks, but in the end decided it didn't > make much sense and scrapped the method. > > My initial approach was to take care of emptiness at the content creation > site, but it soon turned out there are too many sites potentially creating > empty content. After fixing 40+ sites using a new > `Content.addNonEmpty(Content)` method I still got empty elements that > shouldn't be there in the JDK documentation. > > Therefore `HtmlTree.add(Content)` still checks for and drops empty content > arguments. Instead, I added a new `HtmlTree.addUnchecked(Content)` method > that adds the argument without the checks. The new method is only used 5 > times in the whole codebase. It replaces the use of the special > `HtmlTree.EMPTY` content token which is no longer needed. > > A few remarks about the rewritten `HtmlTree.isEmpty()` method: its purpose is > to determine whether content is missing in an element that should have > content. It therefore always returns `false` for elements that are expected > or allowed to be without content. This is maybe a bit counter-intuitive, but > it is what is needed. We could do a combined check such as `isVoid() || > !isEmpty()` but that would still leave out elements that *can* be empty, such > as `<script>`. > > The implementation of `HtmlTree.isEmpty()` is deliberately simple and > conservative. For instance, it doesn't look at attributes to decide whether > an element is allowed to be empty. The rationale for this is to make the > behavior as predictable as possible and avoid surprises. > > Since we no longer have a validity check in `HtmlTree.add(Content)` I had to > remove a few lines of code in `TestHtmlDocument.java` that expected invalid > tags to be dropped. Otherwise there are no test changes, as this is a > noreg-cleanup task. I did compare the javadoc JDK documention before and > after this change, the output is unchanged.
Hannes Wallnöfer has updated the pull request incrementally with one additional commit since the last revision: Introduce Content.isDiscardable() ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/7159/files - new: https://git.openjdk.java.net/jdk/pull/7159/files/3dbbb5aa..406806f3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7159&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7159&range=03-04 Stats: 26 lines in 2 files changed: 15 ins; 3 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/7159.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7159/head:pull/7159 PR: https://git.openjdk.java.net/jdk/pull/7159
