RFR: 8214738 : javadoc should honor styles in doc-files

Fri Dec 21 10:13:50 UTC 2018

Hi Jon,

Thanks for the review.

I have made the following changes:
1)Added style tag in HtmlTag.java, before we were using style attribute 
for comparison.
2)utils.getPreamble(element) ignores new lines after html tags, so I can 
included them while fetching the localHeadTags.
3)In Head.java, used addContent() itself instead of new methods.
I had seen the addContent(), but it was adding the content before the 
scripts/styles tags.
The doc-files script/style needed to added at the end of the other 
styles it that it overrides them.
When I checked the usage of addContent, i see that its been used only by 
IndexRedirectWriter to include script/noscript tags.
I think that can be added at the end of existing tags as well.

updated webrev: http://cr.openjdk.java.net/~pmuthuswamy/8214738/webrev.01/
api: http://cr.openjdk.java.net/~pmuthuswamy/8214738/api/

some of the doc-file with user specific styles.
api/jdk.jdi/com/sun/jdi/doc-files/signature.html
api/java.base/java/lang/doc-files/threadPrimitiveDeprecation.html
api/java.desktop/java/awt/doc-files/Modality.html

Thanks,
Priya

On 12/21/2018 3:16 AM, Jonathan Gibbons wrote:
>
> Priya,
>
> In DocFilesHandlerImpl, you have a boolean flag titleOrMetaTags. Since 
> <meta> elements never have content, and so never have end elements, it 
> seems misleading/incorrect to set the flag to true.  Also, you omit to 
> handle ENTITY in the <title> element, so I added that in as well.  You 
> could also write this code as a visitor if you wanted, but the switch 
> form is not too bad.
>
> I suggest the code should look something like this:
>
> 205 List<DocTree> localTags = new ArrayList<>();  206 boolean inHead = 
> false;
> 207 boolean inTitle = false; loop:   208 for (DocTree dt : dtrees) {
> 209 switch (dt.getKind()) {
> 210 case START_ELEMENT:
> 211 StartElementTree startElem = (StartElementTree) dt; switch 
> (HtmlTag.get(startElem)) { case HEAD: inHead = true; break;case META: 
> break; case TITLE: inTitle=true; break; default: if (inHead) { 
> localTags.add(startElem); } }
> 223 break;   224 case END_ELEMENT:  211 EndElementTree endElem = (EndElementTree) 
> dt; switch (HtmlTag.get(endElem)) { case HEAD: break loop;                               case TITLE:
>                                   inTitle = false;
>                                   break loop;default: if (inHead) { localTags.add(startElem); } }
> 235 break; case ENTITY:   236 case TEXT:
> 237 if (inHead && !inTitle) {
> 238 localTags.add(dt);
>   239                     }
> 240 break;
>   241             }
> -- Jon
>
> On 12/20/2018 11:31 AM, Jonathan Gibbons wrote:
>>
>>
>>
>> On 12/19/2018 08:38 PM, Priya Lakshmi Muthuswamy wrote:
>>> Hi,
>>>
>>> Kindly review the fix for 
>>> https://bugs.openjdk.java.net/browse/JDK-8214738
>>> webrev : http://cr.openjdk.java.net/~pmuthuswamy/8214738/webrev.00/
>>>
>>> Thanks,
>>> Priya
>>>
>>>
>>
>> I don't think you need to add anything (userHeaderTags etc) to Head.
>> Head already has the requisite feature, "extraContent" and "addContent"
>> which can be used to add content into the <head> element.
>>
>> Re: 
>> Utils.toLowerCase(nodeEnd.getName().toString()).equals(HtmlTag.HEAD.getText())
>> Don't use long-winded String comparison when you have type-safe 
>> comparison.
>> In this case, you should be able to do something like
>>     HtmlTag.get(nodeEnd.getName()) == HtmlTag.HEAD
>>
>> getLocalHeaderTags needlessly walks through all the <body> element. 
>> You can
>> break out of the loop once you hit </head>.
>>
>> DocFilesHandlerImpl:196.
>> The "if" statement is a waste of time and effort. The code will work 
>> perfectly
>> well when localTagsContent is an empty list, so you can simplify 
>> 196-199 to just 197.
>>
>> TestCopyFiles.java
>> (style) re: indenting the continuation of multi-line strings
>> The general Java style for breaking long expressions is to break 
>> before a binary operator, not after.
>> The style in other javadoc tests is to not indent the continuation 
>> expressions, so that it is easier
>> to see how the first line and subsequent lines are aligned.
>>
>> -- Jon
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/javadoc-dev/attachments/20181221/c2037384/attachment-0001.html>