RFR: JDK-8239804 : Cleanup/simplify HTML/CSS for general block tags

Wed Feb 26 21:41:05 UTC 2020

Updated webrev, with one additional change.  The new CSS class name is 
changed from "blockTags" to "notes".  This was done automagically in a 
single command, with one manual fixup to move the position of the 
renamed member in the list, which is still alphabetically sorted (for now).

I'm suggesting this rename now, because upcoming work suggests there are 
more definition lists involved that need to be subject to cleanup, for 
which "blockTags" is not a good CSS class name. The proposed name 
"notes" is intended to cover the lists generated for block tags, the 
lists for supertype and subtype information for a class, and the 
supertype information for an overriding method.

New webrev:

Webrev: http://cr.openjdk.java.net/~jjg/8239804/webrev.02/

-- Jon

On 02/25/2020 02:32 PM, Jonathan Gibbons wrote:
> Hannes,
>
> Thanks for all the feedback; comments inline.
>
> New webrev:
>
> Webrev: http://cr.openjdk.java.net/~jjg/8239804/webrev.01/
>
> -- Jon
>
>
> On 02/25/2020 07:05 AM, Hannes Wallnöfer wrote:
>> Hi Jon,
>>
>> I think this is a good change overall. Less HTML tags, less CSS 
>> classes while still providing the same result.
>>
>> A few minor issues:
>>
>> - The new imports in ParamTaglet.java and MethodWriterImpl.java are 
>> not used.
> Fixed.
>>
>> - In line 297 of MethodWriterImpl.java there’s a use of 
>> writer.contents that should use the local contents variable (or get 
>> rid of local var and just use writer.contents)
> I'll keep the local variable, since it's used in a few places, and fix 
> line 297.
>>
>> - In TagletWriter#getParamHeader the javadoc @param tag needs to be 
>> renamed/updated as well.
> Oops, yes. Fixed.
>>
>> - Given its slightly broader usage I’m not fully convinced about the 
>> „blockTags“ name, but can’t think of anything better. I’m fine with 
>> keeping the name for now, maybe we'll find something better in the 
>> upcoming CSS cleanup.
> I'm not wildly enthusiastic about the name either, and like you, I 
> can't think of anything better right now.  But, the only anomaly is 
> the use for overriding methods, and it's hard to come up with a 
> meaningful term for both block tags and overriding info.   There's 
> words like "info", "more info" which are very generic; "details" is 
> too specific and already has a different meaning on the most of the 
> same pages.
>
> There's also the CSS name "block" being used, that means something 
> else, but that is a better candidate to be renamed!
>
>>
>> - I got very confused when I saw one of the CSS styles you removed in 
>> the generated docs in one of the module-overview.html pages. I even 
>> thought the generated docs were built with some other JDK, until I 
>> realised some of the taglets reside outside the src directory. Below 
>> is a patch that updates these taglet classes to conform to the new 
>> output. I think it should be included with this patch for consistency.
>
> Yeah, I know about the other taglets, and was going to handle them 
> separately, but now they've come up here, I'll fix them here as well.
>
>>
>> diff -r 4f902f017def 
>> make/jdk/src/classes/build/tools/taglet/ModuleGraph.java
>> --- a/make/jdk/src/classes/build/tools/taglet/ModuleGraph.java Tue 
>> Feb 25 09:46:12 2020 +0100
>> +++ b/make/jdk/src/classes/build/tools/taglet/ModuleGraph.java Tue 
>> Feb 25 15:32:53 2020 +0100
>> @@ -74,7 +74,7 @@
>>                   + "</span>";
>>           }
>>           return "<dt>"
>> -            + "<span class=\"simpleTagLabel\">Module Graph:</span>\n"
>> +            + "Module Graph:\n"
>>               + "</dt>"
>>               + "<dd>"
>>               + "<a class=moduleGraph href=\"" + imageFile + "\">"
>> diff -r 4f902f017def 
>> make/jdk/src/classes/build/tools/taglet/ToolGuide.java
>> --- a/make/jdk/src/classes/build/tools/taglet/ToolGuide.java Tue Feb 
>> 25 09:46:12 2020 +0100
>> +++ b/make/jdk/src/classes/build/tools/taglet/ToolGuide.java Tue Feb 
>> 25 15:32:53 2020 +0100
>> @@ -95,7 +95,7 @@
>>               return "";
>>             StringBuilder sb = new StringBuilder();
>> -        sb.append("<dt class=\"simpleTagLabel\">Tool Guides:</dt>\n")
>> +        sb.append("<dt>Tool Guides:</dt>\n")
>>                   .append("<dd>");
>>             boolean needComma = false;
>>
>>
>> —
>> Hannes
>>
>>
>>> Am 22.02.2020 um 01:57 schrieb Jonathan Gibbons 
>>> <jonathan.gibbons at oracle.com>:
>>>
>>> Please review a change to cleanup/simplify the HTML and CSS used to 
>>> present block tags, like @see, @since, etc.
>>>
>>> Currently, the information from these tags is presented in a 
>>> definition list (<dl>). The labels, in the <dt> nodes, are wrapped 
>>> in <span> using an inconsistent set of CSS class names. Sometimes 
>>> the names are shared, sometimes they are unique.
>>>
>>> With this change, a new CSS class "blockTags" is put on the 
>>> enclosing <dl> node, and the <span> nodes wrapping the individual 
>>> labels removed. This does mean that it is no longer possible to 
>>> style some of the labels individually, but the use of CSS class 
>>> names that were sometimes shared, sometime unique, makes it unlikely 
>>> that anyone tried to do this. If (and only if) there is a demand to 
>>> style tags individually, that should be handled as a new Enhancement 
>>> request, that might involve putting tag-specific CSS class names on 
>>> the <dt> and <dd> nodes for each tag (i.e. without an additional 
>>> <span>).
>>>
>>> There is one anomaly. The definition list for the block tags for a 
>>> method is also used to present information when the method overrides 
>>> or implements a method from a supertype. That makes the new CSS 
>>> class name of "blockTags" slightly less than ideal. Suggestions for 
>>> a better name are welcome.
>>>
>>> The code to generate the overall list of tags for any element is not 
>>> as centralized as might be desired. This changeset does not attempt 
>>> to fix that.
>>>
>>> There should be no visible change to docs generated using the 
>>> default stylesheet when the docs are viewed in a browser.
>>>
>>> The src/ code changes are relatively simple, and just consist of 
>>> adjusting the HTML and CSS that is generated. In some cases, there 
>>> is some additional code cleanup. About 30 tests are affected, 
>>> although the changes are generally simple and localized. The bug 
>>> number has been added to the @bug list for a subset of the tests; 
>>> there are no new additional tests.
>>>
>>> ---
>>>
>>> A separate exercise would be to identify other <dl> nodes generated 
>>> by the doclet, and to add a CSS class to those nodes to identify the 
>>> kind of list.
>>>
>>> -- Jon
>>>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8239804
>>> Webrev: http://cr.openjdk.java.net/~jjg/8239804/webrev.00/
>>>
>