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

[Priya Lakshmi Muthuswamy]

Hi Jon,

Javadoc styles for header/navbar will appear as it is and will not get 
disturbed.
I meant for any html content in doc-files, if there is any user-defined 
style, then those should override the javadoc styles.
In html head element, default(javadoc) styles will be placed first and 
then those defined in doc-file.

Thanks,
Priya

On 12/21/2018 9:24 PM, Jonathan Gibbons wrote:

> I've not yet read the full webrev, but I do have one immediate 
> question, given below.
>
> -- Jon
>
> On 12/21/18 2:13 AM, Priya Lakshmi Muthuswamy wrote:
>>
>> 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.
>>
> This is a red flag.  Why is it important that the user styles override 
> the javadoc styles?  Given that we're adding javadoc styles for the 
> javadoc header/navbar, isn't it important that those should appear as 
> intended?
>
>
>> 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/20181226/b947da6a/attachment.html>