RFR: JDK-8275788: Create code element with suitable attributes for code snippets [v3]

[Hannes Wallnöfer]

On Fri, 29 Oct 2021 09:18:48 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> Hannes Wallnöfer has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
>> 
>>   Avoid use of String.toLowerCase() without locale and reverse renaming artefact
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TagletWriterImpl.java line 385:
> 
>> 383:     protected Content snippetTagOutput(Element element, SnippetTree tag, StyledText content) {
>> 384:         HtmlTree result = new HtmlTree(TagName.PRE).setStyle(HtmlStyle.snippet);
>> 385:         result.add(Text.of(utils.normalizeNewlines("\n")));
> 
>> We previously added a newline character to the snippet's `<pre>` element. I think this was done in order to make sure the element is rendered even if the snippet content is empty. This was fine with just the `<pre>` element, but with a nested `<code>` element that newline is visible in the browser. I therefore replaced the newline with `HtmlTree.EMPTY`.
> 
> This is not why that newline was added. The rationale for adding an unconditional newline that immediately follows `<pre>` was twofold.
> 
> Firstly, the HTML rules for the `pre` element are quite special:
> 
> - https://html.spec.whatwg.org/multipage/grouping-content.html#the-pre-element
> - https://html.spec.whatwg.org/multipage/syntax.html#element-restrictions
> 
> Secondly, starting a snippet from a new line looks better in HTML and resembles its appearance in the browser.
> 
> If you are sure that those reasons are no longer valid because of the changed structure, by all means remove that newline.

Thanks for the pointers, I didn't know this although I realized there had to be special rules with leading newlines in `<pre>` content. The rule does not apply to the content of a `<code>` element inside a `<pre>`. We could keep the newline *before* the `<code>` element, but it is no longer needed so I think it can be removed.

> test/langtools/jdk/javadoc/doclet/testSnippetTag/TestSnippetTag.java line 137:
> 
>> 135: //                new SnippetAttributes("""
>> 136: //                    {@snippet id=foo6:
>> 137: //                        Hello, SnippetAttributes!
> 
> Looks like a renaming artefact.

Good catch and you guessed correctly. I reversed it.

-------------

PR: https://git.openjdk.java.net/jdk/pull/6165