Hi Jon, Looks good.
Sorry for the late review, there’s still a lot of new code for me to explore. Hannes > Am 20.11.2018 um 01:37 schrieb Jonathan Gibbons <[email protected]>: > > Ooops, I forgot the links. > > JBS: https://bugs.openjdk.java.net/browse/JDK-8213956 > webrev: http://cr.openjdk.java.net/~jjg/8213956/webrev.00/ > > -- Jon > > On 11/16/2018 12:10 PM, Jonathan Gibbons wrote: >> Please review a moderately simple fix to a javadoc crash that was caused by >> using {@index} in an HTML file in a doc-files subdirectory. >> >> The root cause is that the standard doclet uses custom elements when >> processing HTML files in doc-files subdirectories and any overview file. >> These custom elements were causing the viistUnknown to be called in a >> visitor that is used while processing {@index} tags. The basic fix is >> therefore to implement visitUnknown. >> >> • TagletWriterImpl: The "hard" part of the fix was determining the URL >> for the target of the search link, and in particular, determining the path >> in that URL. However, once found, the solution was easy: the taglet writer >> has access to the enclosing HTML writer, which knows the file that is being >> written. Since this is true for the use of {@index} in the documentation >> for all elements, we can simplify the visitor by setting the URL outside >> the visitor. And then, since many of the visit methods end up having the >> same functionality as the default action (i.e. using the fully qualified >> name of the element), we can delete those overriding methods and use the >> defaultAction method instead. >> >> The changes also fix another minor issue: the simple name was used instead >> of the fully qualified name of the package for the "holder" of the search >> item. See line 437 in the "before" version of TagletWriterImpl.java >> >> • HTMLDoclet: Although the crash was in doc-files, the code should >> support {@index} in the overview file. But for that to work, the index files >> must be generated after the overview file has been processed. >> >> • DocFilesHandlerImpl: Fix the reporting of the file being generated: >> it was printing the default .toString() and not the actual path of the file. >> >> • New test: the new test verifies the use of {@index} in >> package-info.java files, files in doc-files subdirectories, and in an >> overview file. Since {@systemProperty} is a related new feature, the test >> also verifies the use of {@systemProperty} in package-info.java and files in >> doc-files subdirectories. (The tag is not permitted in overview files.) >> All javadoc tests pass; in addition, the .html files in JDK API >> documentation are not affected by the cleanup, and compare the same, before >> and after. >> >> -- Jon >> >
