RFR: JDK-8240136: Cleanup/simplify HTML/CSS for definition lists

[Jonathan Gibbons]

Thanks for all the feedback.

I'm surprised I missed that occurrence of Overriden.  I thought I caught 
all instances. I will fix.

-- Job


On 2/28/20 7:25 AM, Pavel Rappo wrote:
>> On 27 Feb 2020, at 21:11, Jonathan Gibbons <jonathan.gibbons at oracle.com> wrote:
>>
>> Please review the next round of update for definition lists in the generated docs.
>>
>> The primary focus of this round is to ensure that all <dl> elements have the class attribute set.
>>
>> In practice, there are only two kinds of definition lists:
>>
>> 	• 1. the kind we now know as "notes", (where the <dt> elements are bold and the <dd> elements not indented)
>> 	• 2. the entries in the index files.These previously did not have a class attribute set, and so used the default style for definition lists; these lists now have the class attribute set to "index", although no specific style is defined for "index" at this point, and so they continue to use the default style for definition lists.
>> By defining the class attribute on all definition lists, we set up being able to simplify the stylesheet (in the next webrev) to remove contentContainer nodes and friends.
> For the record, there seems to be at least one other type, "nameValue",
> used in serialized-form.html.
>
>> The edits to the source files are generally simple, and mostly consist of using new/improved factory methods for <dl> nodes that take a style parameter.
> "Naked" calls gave way to convenience static methods that force you to use
> "style", good.
>
> Before:
>
>      HtmlTree.DL(paramInfo).setStyle(HtmlStyle.notes)
>
>    or even more mouthful
>
>      new HtmlTree(HtmlTag.DL).setStyle(HtmlStyle.notes)
>
> After:
>
>      HtmlTree.DL(HtmlStyle.notes, paramInfo)
>
>> I have also done some local reordering of the code to setup for another cleanup down the road.  Previously, the code pattern was
>>
>> 	• a. create the child <dt> node
>> 	• b. create the parent <dl> node containing the <dt> node
>> 	• c. create the child <dd> node and add it to the <dl> node
>> This was in response to a concern a long, long time ago about creating and generating empty nodes: so the code style was to try and create nodes with content, even though it led to the confusing sequence I just described.
> Huh, I would've never guessed!
>
>>   The new sequence is:
>>
>> 	• a. create the parent <dl> node
>> 	• b. create the child <dt> node and add it to the <dl> node
>> 	• c. create the child <dd> node and add it to the <dl> node
>> The reordering sets up the option to introduce and use chained method calls in future, to simplify the code even more, but that was too much of a change to add in this changeset.
> Thanks, it looks neat.
>
>> There's a bunch of updates I could (and want to) make to the comments throughout the HtmlTree class, but I think they should be done separately so that the file has a consistent comment style.
> +1 for splitting the cleanup.
>
>> One other item that became clear in this work is the use of "singleton lists" in which there are a number of instances where the <dl> node is always created with a single <dt>/<dd> pair. Moreover, a sequence of these singleton lists may occur, depending on the characteristics of the class being documented. This changeset does _not_ address that issue, which should be handled separately. This is the definition-list equivalent of the "sequence of singleton lists" that we fixed for <ul> lists a while back!
>>
>> The tests are generally just updated to expect the class attribute to be set in <dl> elements.
>>
>> -- Jon
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8240136
>> Webrev: http://cr.openjdk.java.net/~jjg/8240136/webrev.00/index.html
> As always, thanks for improvements on the go such as fixing typos, broken indentation, etc.
>
> If it is still possible, could you please fix the spelling in the name of the
> "testExternalOverridenMethod" folder? I think we missed that yesterday while
> reviewing (now pushed) http://hg.openjdk.java.net/jdk/jdk/rev/52367bbd4bd4.
>
> Looks good to me.
>
> -Pavel
>