On Fri, 29 Oct 2021 09:18:48 GMT, Pavel Rappo <[email protected]> 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
