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
