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

[Pavel Rappo]

On Thu, 28 Oct 2021 21:02:04 GMT, Hannes Wallnöfer <hannesw at openjdk.org> wrote:

> Please review a change to add a nested `<code>` element for snippet tags as well as HTML attributes for the snippet's `id` and `lang` attributes. The change is quite simple, I did however encounter some gotchas. Below is a short list of notes to explain some aspects of the change.
> 
>  - Both `id` and `lang` are only meaningful if they have non-empty values, so defining them with empty or missing values will cause the respective HTML attributes to be not defined.
>  - I decided to put the logic for extracting the `id` and `lang` attributes into the already quite long `SnippetTaglet.getInlineTagOutput` method as this is where the required preparation and infrastructure are located. Both values are then passed as additional arguments to `TagletWriter.snippetTagOutput` which is not great. I think it's ok for now, but maybe if we need to pass more values along that way we should reconsider this design.
>  - I'm not sure whether we should assume `lang=java` for internal snippets. This change currently does not, it only derives implicit `lang` attributes for external snippets based on the extension of the snippet source file.
>  - In `SnippetTaglet.languageFromFilename()` I tried to cover what I reckoned would be the most useful languages for javadoc users, including a few major JVM languages. I'm sure I forgot some, but I hope I got at least those file extensions right.
>  - 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`. 
>  - For the test, I was able to convert the existing `testIdAndLangAttributes` test to cover explicit `id` and `lang` attributes. I added a new test called `testExternalImplicitAttributes` to test derived `lang` attributes.

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.

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

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