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