from:"Hannes Wallnöfer"

On Wed, 7 Aug 2024 21:32:44 GMT, Jonathan Gibbons  wrote:

> One question/suggestion, perhaps for followup work. I get that you do not 
> want the visual clutter of linking to locally defined type parameters, but 
> would it help to give a short tooltip (i.e. `title` attribute) in this 
> situation even if you do not also create a link. Is this an interesting idea 
> for any place where we cannot or do not create a link?

I like this idea, and it's quite tempting to add it to this PR since I had 
already implemented it and it's only a few lines of code. 

I wonder if we should go the extra mile and include the element kind in the 
tooltip message (`type parameter in interface [name]` or `type parameter in 
method [name]()` instead of just `type parameter in [name]`).

-

PR Comment: https://git.openjdk.org/jdk/pull/20494#issuecomment-2275048609

Re: RFR: 8313931: Javadoc: links to type parameters actually generate links to classes [v3]

> Please review a JavaDoc change to make it possible to link to type parameter 
> documentation from automatically generated signatures as well as user-defined 
> `{@link ...}` and `@see ...` tags. The solution consists in wrapping type 
> parameter documentation into `` 
> elements for generic classes and ` id="[member-signature]-type-param-[type-param-name]">` for generic members. 
> 
> While this is not a very big change, there are a few aspects and 
> considerations that need to be explained.
> 
> - Class-level type parameter documentation had to be moved out of the ` class="horizontal-scroll">` element for the main class description because of 
> [this bug in Chrome](https://issues.chromium.org/issues/40074749). The only 
> viable solution was to move the scroll container beneath the `` element, 
> which is the way it is already done for package and module pages. This has 
> almost no visual effects on the way pages are rendered, I'd be happy to 
> discuss the stylesheet tweaks if there's interest.
> 
> - JavaDoc only creates links for type parameters in generic types, but not 
> those belonging to generic methods and constructors. While I added support 
> for linking member-level type parameters, I soon realized that these links 
> add a lot of visual noise by adding redundant links from member-level 
> descriptions and signatures to the details of the same member. Displaying 
> only type-level type parameters as links is actually quite helpful in 
> allowing to distinguish them from member-level type parameters. The 
> [documentation of 
> `java.util.Map`](https://download.java.net/java/early_access/jdk24/docs/api/java.base/java/util/Map.html#method-summary)
>  illustrates this nicely. The solution I settled for is to allow linking to 
> member-level type parameters using `{@link ...}` and `@see ...` tags if a 
> `@param` tag is provided for the type parameter, but not create such links 
> for automatically generated signatures.
> 
> - Since `javadoc` will always create links for type parameters of generic 
> types, a link target with the appropriate `id` attribute has to be created 
> even if no `@param` tag for the type parameter is provided. This is done by 
> creating a span with the given `id` in the type signature shown in the main 
> heading of the page. 
> 
> - When type parameter documentation is displayed as link target, it is 
> rendered with a visual highlight, which is similar to the one used for 
> `{@index ...}` tags but fades out after a few seconds. The purpose is to make 
> the type par...

Hannes Wallnöfer has updated the pull request incrementally with one additional 
commit since the last revision:

  More post-merge cleanup

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/20494/files
  - new: https://git.openjdk.org/jdk/pull/20494/files/ffd4eccf..bfc7c7dd

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=20494&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20494&range=01-02

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/20494.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/20494/head:pull/20494

PR: https://git.openjdk.org/jdk/pull/20494

Re: RFR: 8313931: Javadoc: links to type parameters actually generate links to classes [v2]

> Please review a JavaDoc change to make it possible to link to type parameter 
> documentation from automatically generated signatures as well as user-defined 
> `{@link ...}` and `@see ...` tags. The solution consists in wrapping type 
> parameter documentation into `` 
> elements for generic classes and ` id="[member-signature]-type-param-[type-param-name]">` for generic members. 
> 
> While this is not a very big change, there are a few aspects and 
> considerations that need to be explained.
> 
> - Class-level type parameter documentation had to be moved out of the ` class="horizontal-scroll">` element for the main class description because of 
> [this bug in Chrome](https://issues.chromium.org/issues/40074749). The only 
> viable solution was to move the scroll container beneath the `` element, 
> which is the way it is already done for package and module pages. This has 
> almost no visual effects on the way pages are rendered, I'd be happy to 
> discuss the stylesheet tweaks if there's interest.
> 
> - JavaDoc only creates links for type parameters in generic types, but not 
> those belonging to generic methods and constructors. While I added support 
> for linking member-level type parameters, I soon realized that these links 
> add a lot of visual noise by adding redundant links from member-level 
> descriptions and signatures to the details of the same member. Displaying 
> only type-level type parameters as links is actually quite helpful in 
> allowing to distinguish them from member-level type parameters. The 
> [documentation of 
> `java.util.Map`](https://download.java.net/java/early_access/jdk24/docs/api/java.base/java/util/Map.html#method-summary)
>  illustrates this nicely. The solution I settled for is to allow linking to 
> member-level type parameters using `{@link ...}` and `@see ...` tags if a 
> `@param` tag is provided for the type parameter, but not create such links 
> for automatically generated signatures.
> 
> - Since `javadoc` will always create links for type parameters of generic 
> types, a link target with the appropriate `id` attribute has to be created 
> even if no `@param` tag for the type parameter is provided. This is done by 
> creating a span with the given `id` in the type signature shown in the main 
> heading of the page. 
> 
> - When type parameter documentation is displayed as link target, it is 
> rendered with a visual highlight, which is similar to the one used for 
> `{@index ...}` tags but fades out after a few seconds. The purpose is to make 
> the type par...

Hannes Wallnöfer has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains four commits:

 - Actually resolve conflicts
 - Merge branch 'master' into JDK-8313931
 - Remove trailing whitespace
 - JDK-8313931: Javadoc: links to type parameters actually generate links to 
classes

-

Changes: https://git.openjdk.org/jdk/pull/20494/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20494&range=01
  Stats: 348 lines in 26 files changed: 257 ins; 12 del; 79 mod
  Patch: https://git.openjdk.org/jdk/pull/20494.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/20494/head:pull/20494

PR: https://git.openjdk.org/jdk/pull/20494

RFR: 8313931: Javadoc: links to type parameters actually generate links to classes

Please review a JavaDoc change to make it possible to link to type parameter
documentation from automatically generated signatures as well as user-defined
`{@link ...}` and `@see ...` tags. The solution consists in wrapping type
parameter documentation into ``
elements for generic classes and `` for generic members.

While this is not a very big change, there are a few aspects and considerations
that need to be explained.

- Class-level type parameter documentation had to be moved out of the `` element for the main class description because of
[this bug in Chrome](https://issues.chromium.org/issues/40074749). The only
viable solution was to move the scroll container beneath the `` element,
which is the way it is already done for package and module pages. This has
almost no visual effects on the way pages are rendered, I'd be happy to discuss
the stylesheet tweaks if there's interest.

- JavaDoc only creates links for type parameters in generic types, but not
those belonging to generic methods and constructors. While I added support for
linking member-level type parameters, I soon realized that these links add a
lot of visual noise by adding redundant links from member-level descriptions
and signatures to the details of the same member. Displaying only type-level
type parameters as links is actually quite helpful in allowing to distinguish
them from member-level type parameters. The [documentation of
`java.util.Map`](https://download.java.net/java/early_access/jdk24/docs/api/java.base/java/util/Map.html#method-summary)
illustrates this nicely. The solution I settled for is to allow linking to
member-level type parameters using `{@link ...}` and `@see ...` tags if a
`@param` tag is provided for the type parameter, but not create such links for
automatically generated signatures.

- Since `javadoc` will always create links for type parameters of generic
types, a link target with the appropriate `id` attribute has to be created even
if no `@param` tag for the type parameter is provided. This is done by creating
a span with the given `id` in the type signature shown in the main heading of
the page.

- When type parameter documentation is displayed as link target, it is rendered
with a visual highlight, which is similar to the one used for `{@index ...}`
tags but fades out after a few seconds. The purpose is to make the type
parameter documentation recognizable as link target. While we found that a
temporary highlight is sufficient for this use case, `{@index ...}` and related
tags continue to use a permanent highlight.

I wanted to upload sample documentation to `cr.openjdk.org`, but unfortunately
I'm currently unable to connect to the server. I'll try again later.

Commit messages:
- Remove trailing whitespace
- JDK-8313931: Javadoc: links to type parameters actually generate links to
classes

Changes: https://git.openjdk.org/jdk/pull/20494/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20494&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8313931
Stats: 348 lines in 26 files changed: 257 ins; 12 del; 79 mod
Patch: https://git.openjdk.org/jdk/pull/20494.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/20494/head:pull/20494

PR: https://git.openjdk.org/jdk/pull/20494

Re: RFR: 8335122: Reorganize internal low-level support for HTML in jdk.javadoc [v9]

2024-08-02 Thread Hannes Wallnöfer

On Fri, 2 Aug 2024 17:26:48 GMT, Jonathan Gibbons  wrote:

>> Please review a change to reorganize the internal low-level support for HTML 
>> in the jdk.javadoc module.
>> 
>> Hitherto, there are two separate sets of classes for low-level support for 
>> HTML in the `jdk.javadoc` module: one, in doclint, focused on reading and 
>> checking classes, the other, in the standard doclet, focused on generating 
>> HTML. This PR merges those two sets, into a new package 
>> `jdk.javadoc.internal.html` that is now used by both `doclint` and the 
>> standard doclet.
>> 
>> There was a naming "anti-clash" -- `HtmlTag` in `doclint` vs `TagName` in 
>> the standard doclet. The resolution is to use `HtmlTag`, since the merged 
>> class is more than just the tag name.
>> 
>> A few minor bugs were found and fixed.   Other minor cleanup was done, but 
>> otherwise, there should be no big surprises here. But, one small item of 
>> note: `enum HtmlStyle` was split into `interface HtmlStyle` and `enum 
>> HtmlStyles implements HtmlStyle` to avoid having a doclet-specific enum 
>> class in the new `internal.html` package.  The naming follows `HtmlId` and 
>> `HtmlIds`.
>> 
>> There is no attempt at this time to simplify `HtmlTag` and `HtmlAttr` to 
>> remove support for older versions of HTML.
>
> Jonathan Gibbons has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Fix trailing whitespace

Thanks for adding the comments, looks good to me.

-

Marked as reviewed by hannesw (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19916#pullrequestreview-2216095763

Re: RFR: 8335122: Reorganize internal low-level support for HTML in jdk.javadoc [v7]

2024-08-01 Thread Hannes Wallnöfer

On Wed, 31 Jul 2024 18:00:11 GMT, Jonathan Gibbons  wrote:

>> Please review a change to reorganize the internal low-level support for HTML 
>> in the jdk.javadoc module.
>> 
>> Hitherto, there are two separate sets of classes for low-level support for 
>> HTML in the `jdk.javadoc` module: one, in doclint, focused on reading and 
>> checking classes, the other, in the standard doclet, focused on generating 
>> HTML. This PR merges those two sets, into a new package 
>> `jdk.javadoc.internal.html` that is now used by both `doclint` and the 
>> standard doclet.
>> 
>> There was a naming "anti-clash" -- `HtmlTag` in `doclint` vs `TagName` in 
>> the standard doclet. The resolution is to use `HtmlTag`, since the merged 
>> class is more than just the tag name.
>> 
>> A few minor bugs were found and fixed.   Other minor cleanup was done, but 
>> otherwise, there should be no big surprises here. But, one small item of 
>> note: `enum HtmlStyle` was split into `interface HtmlStyle` and `enum 
>> HtmlStyles implements HtmlStyle` to avoid having a doclet-specific enum 
>> class in the new `internal.html` package.  The naming follows `HtmlId` and 
>> `HtmlIds`.
>> 
>> There is no attempt at this time to simplify `HtmlTag` and `HtmlAttr` to 
>> remove support for older versions of HTML.
>
> Jonathan Gibbons has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 12 commits:
> 
>  - Merge with upstream master;
>add reference in HtmlTag to a related RFE
>  - Update/improve merge of HtmlTag and TagName
>  - Fix missing legal headers
>  - Cleanup use of HtmlStyle and HtmlStyles
>  - Fix merge issues
>  - Merge with upstream master
>  - fix whitespace
>  - reorder imports
>  - Cleanup pass
>  - Merge doclint into html: part 2: TagName/HtmlTag
>  - ... and 2 more: https://git.openjdk.org/jdk/compare/e4c7850c...2b6363ee

Looks good! It would be nice to have class-level doc comments in the new shared 
HTML classes.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/html/HtmlAttr.java line 32:

> 30: import java.util.Locale;
> 31: 
> 32: public enum HtmlAttr {

Not essential, but it would be nice to have a simple class doc comment such as 
"Enum representing HTML tag attributes."

src/jdk.javadoc/share/classes/jdk/javadoc/internal/html/HtmlStyle.java line 28:

> 26: package jdk.javadoc.internal.html;
> 27: 
> 28: public interface HtmlStyle {

Again, a simple doc comment would be nice.

-

Marked as reviewed by hannesw (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19916#pullrequestreview-2212514448
PR Review Comment: https://git.openjdk.org/jdk/pull/19916#discussion_r1700013819
PR Review Comment: https://git.openjdk.org/jdk/pull/19916#discussion_r1700014922

Re: RFR: 8335122: Reorganize internal low-level support for HTML in jdk.javadoc [v4]

2024-07-26 Thread Hannes Wallnöfer

On Wed, 24 Jul 2024 22:09:46 GMT, Jonathan Gibbons  wrote:

>> Please review a change to reorganize the internal low-level support for HTML 
>> in the jdk.javadoc module.
>> 
>> Hitherto, there are two separate sets of classes for low-level support for 
>> HTML in the `jdk.javadoc` module: one, in doclint, focused on reading and 
>> checking classes, the other, in the standard doclet, focused on generating 
>> HTML. This PR merges those two sets, into a new package 
>> `jdk.javadoc.internal.html` that is now used by both `doclint` and the 
>> standard doclet.
>> 
>> There was a naming "anti-clash" -- `HtmlTag` in `doclint` vs `TagName` in 
>> the standard doclet. The resolution is to use `HtmlTag`, since the merged 
>> class is more than just the tag name.
>> 
>> A few minor bugs were found and fixed.   Other minor cleanup was done, but 
>> otherwise, there should be no big surprises here. But, one small item of 
>> note: `enum HtmlStyle` was split into `interface HtmlStyle` and `enum 
>> HtmlStyles implements HtmlStyle` to avoid having a doclet-specific enum 
>> class in the new `internal.html` package.  The naming follows `HtmlId` and 
>> `HtmlIds`.
>> 
>> There is no attempt at this time to simplify `HtmlTag` and `HtmlAttr` to 
>> remove support for older versions of HTML.
>
> Jonathan Gibbons has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Cleanup use of HtmlStyle and HtmlStyles

src/jdk.javadoc/share/classes/jdk/javadoc/internal/html/HtmlTag.java line 87:

> 85: attrs(AttrKind.HTML4, CLEAR)),
> 86: 
> 87: BUTTON(BlockType.OTHER, EndKind.REQUIRED,

Several tag constants that use `BlockType.OTHER` in this enum are defined as 
[Phrasing Content](https://html.spec.whatwg.org/#phrasing-content) in the HTML5 
spec. Since HTML5 phrasing content roughly corresponds to pre-HTML5 inline 
content these tags should use `BlockType.INLINE` here. This includes the 
following tags:

 - BUTTON
 - INPUT
 - LABEL
 - LINK
 - SCRIPT

These tags were also flagged as `phrasingContent` in the old doclet `TagName` 
enum. I'm not sure whether marking it as `INLINE` content will break DocLint 
tests.

It would seem like a good idea to suggest using [HTML5 content 
categories](https://developer.mozilla.org/en-US/docs/Web/HTML/Content_categories)
 in the new merged code, but the new categories are more complex and 
overlapping, and don't include list and table content, so there is not a lot to 
gain besides maybe more up-to-date terminology.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19916#discussion_r1692888920

Re: RFR: 8335122: Reorganize internal low-level support for HTML in jdk.javadoc [v4]

2024-07-25 Thread Hannes Wallnöfer

On Wed, 24 Jul 2024 22:09:46 GMT, Jonathan Gibbons  wrote:

>> Please review a change to reorganize the internal low-level support for HTML 
>> in the jdk.javadoc module.
>> 
>> Hitherto, there are two separate sets of classes for low-level support for 
>> HTML in the `jdk.javadoc` module: one, in doclint, focused on reading and 
>> checking classes, the other, in the standard doclet, focused on generating 
>> HTML. This PR merges those two sets, into a new package 
>> `jdk.javadoc.internal.html` that is now used by both `doclint` and the 
>> standard doclet.
>> 
>> There was a naming "anti-clash" -- `HtmlTag` in `doclint` vs `TagName` in 
>> the standard doclet. The resolution is to use `HtmlTag`, since the merged 
>> class is more than just the tag name.
>> 
>> A few minor bugs were found and fixed.   Other minor cleanup was done, but 
>> otherwise, there should be no big surprises here. But, one small item of 
>> note: `enum HtmlStyle` was split into `interface HtmlStyle` and `enum 
>> HtmlStyles implements HtmlStyle` to avoid having a doclet-specific enum 
>> class in the new `internal.html` package.  The naming follows `HtmlId` and 
>> `HtmlIds`.
>> 
>> There is no attempt at this time to simplify `HtmlTag` and `HtmlAttr` to 
>> remove support for older versions of HTML.
>
> Jonathan Gibbons has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Cleanup use of HtmlStyle and HtmlStyles

Two files in the new `html` package are missing copyright header and doc 
comments: `HtmlAttr.java` and `HtmlStyle.java`.

-

PR Comment: https://git.openjdk.org/jdk/pull/19916#issuecomment-2250071458

Re: RFR: 8335122: Reorganize internal low-level support for HTML in jdk.javadoc [v4]

2024-07-25 Thread Hannes Wallnöfer

On Wed, 24 Jul 2024 22:09:46 GMT, Jonathan Gibbons  wrote:

>> Please review a change to reorganize the internal low-level support for HTML 
>> in the jdk.javadoc module.
>> 
>> Hitherto, there are two separate sets of classes for low-level support for 
>> HTML in the `jdk.javadoc` module: one, in doclint, focused on reading and 
>> checking classes, the other, in the standard doclet, focused on generating 
>> HTML. This PR merges those two sets, into a new package 
>> `jdk.javadoc.internal.html` that is now used by both `doclint` and the 
>> standard doclet.
>> 
>> There was a naming "anti-clash" -- `HtmlTag` in `doclint` vs `TagName` in 
>> the standard doclet. The resolution is to use `HtmlTag`, since the merged 
>> class is more than just the tag name.
>> 
>> A few minor bugs were found and fixed.   Other minor cleanup was done, but 
>> otherwise, there should be no big surprises here. But, one small item of 
>> note: `enum HtmlStyle` was split into `interface HtmlStyle` and `enum 
>> HtmlStyles implements HtmlStyle` to avoid having a doclet-specific enum 
>> class in the new `internal.html` package.  The naming follows `HtmlId` and 
>> `HtmlIds`.
>> 
>> There is no attempt at this time to simplify `HtmlTag` and `HtmlAttr` to 
>> remove support for older versions of HTML.
>
> Jonathan Gibbons has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Cleanup use of HtmlStyle and HtmlStyles

Changes requested by hannesw (Reviewer).

src/jdk.javadoc/share/classes/jdk/javadoc/internal/html/HtmlAttr.java line 25:

> 23:  * questions.
> 24:  */
> 25: 

Copyright header and doc comments were removed here (I guess by accident).

src/jdk.javadoc/share/classes/jdk/javadoc/internal/html/HtmlStyle.java line 5:

> 3: public interface HtmlStyle {
> 4: String cssName();
> 5: }

Copyright header and doc comments are missing in this file.

-

PR Review: https://git.openjdk.org/jdk/pull/19916#pullrequestreview-2198994299
PR Review Comment: https://git.openjdk.org/jdk/pull/19916#discussion_r1691263216
PR Review Comment: https://git.openjdk.org/jdk/pull/19916#discussion_r1691261610

Re: RFR: 6381729: Javadoc for generic constructor doesn't document type parameter

2024-07-23 Thread Hannes Wallnöfer

On Mon, 22 Jul 2024 15:08:02 GMT, Archie Cobbs  wrote:

>> This needs to be coordinated with @hns who is working on similar issues. He 
>> is currently OOTO, but will be back soon.
>
>> This needs to be coordinated with @hns who is working on similar issues. He 
>> is currently OOTO, but will be back soon.
> 
> @hns are you back in town? Any thoughts/comments? Thanks.

Thanks for the heads-up, @archiecobbs and @jonathan-gibbons. The only file I 
also modified in my current patch is `AbstractExectutableMemberWriter.java`, 
but I modify existing code whereas you add a method, so no problems there.

-

PR Comment: https://git.openjdk.org/jdk/pull/20044#issuecomment-2245252422

Re: RFR: 8335122: Reorganize internal low-level support for HTML in jdk.javadoc

2024-07-08 Thread Hannes Wallnöfer

On Wed, 26 Jun 2024 23:00:27 GMT, Jonathan Gibbons  wrote:

> Please review a change to reorganize the internal low-level support for HTML 
> in the jdk.javadoc module.
> 
> Hitherto, there are two separate sets of classes for low-level support for 
> HTML in the `jdk.javadoc` module: one, in doclint, focused on reading and 
> checking classes, the other, in the standard doclet, focused on generating 
> HTML. This PR merges those two sets, into a new package 
> `jdk.javadoc.internal.html` that is now used by both `doclint` and the 
> standard doclet.
> 
> There was a naming "anti-clash" -- `HtmlTag` in `doclint` vs `TagName` in the 
> standard doclet. The resolution is to use `HtmlTag`, since the merged class 
> is more than just the tag name.
> 
> A few minor bugs were found and fixed.   Other minor cleanup was done, but 
> otherwise, there should be no big surprises here. But, one small item of 
> note: `enum HtmlStyle` was split into `interface HtmlStyle` and `enum 
> HtmlStyles implements HtmlStyle` to avoid having a doclet-specific enum class 
> in the new `internal.html` package.  The naming follows `HtmlId` and 
> `HtmlIds`.
> 
> There is no attempt at this time to simplify `HtmlTag` and `HtmlAttr` to 
> remove support for older versions of HTML.

I'm not sure if this is intentional (it makes changes easier to review), but I 
notice that imports of classes in the new package are generally mixed with 
classes remaining in the old package, and therefore not in alphabetic order.

-

PR Review: https://git.openjdk.org/jdk/pull/19916#pullrequestreview-2148044997

Re: RFR: 8325690: The scrollable element with non-interactive content is not tabbable [v38]

2024-07-02 Thread Hannes Wallnöfer

On Mon, 24 Jun 2024 08:04:08 GMT, psoujany  wrote:

>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Table.java
>>  line 341:
>> 
>>> 339: cell.addStyle(tabClass);
>>> 340: if (!matchFound) {
>>> 341: cell.put(HtmlAttr.ROLE, "row")
>> 
>> We now use [CSS Grid 
>> layout](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_grid_layout) 
>> for tables. Unfortunately, this implies that we do not have HTML elements 
>> representing rows. Tables are represented by an unstructured stream of table 
>> cells. This means that 
>> [`role="cell"`](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/cell_role)
>>  would probably be the right attribute to use here.
>> 
>> Once 
>> [Subgrid](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Grid_Layout/Subgrid)
>>  is well enough supported in browsers (which may be soon), we will be able 
>> to introduce elements representing table rows.
>
> @hns 
> [role="cell"](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/cell_role)
>  should be used within row element. But the code here has row styles as 
> `col-first even-row-color` and `col-last even-row-color` and also tabindex 
> should be added to only widget role i.e, `role="row"`. In order to add 
> `role="cell"` and `role="row"` ARIA elements then we have to restructure the 
> existing code. Could you please suggest here.
> 
> **Current code generates docs as :**
> 
>  
> Some Link
> 
> 
> 
> **To add roles row and cell, docs to be generated as :**
> 
> 
> SomeLink
> 
> 
>  
> 

The problem I see is that we do not currently have elements representing table 
rows. There is [a JBS issue](https://bugs.openjdk.org/browse/JDK-8288900) for 
this problem, and it looks like we might be able to solve it in the not too 
distant future, depending on browsers supporting the necessary CSS features.  
However, until we do have actual elements representing table rows, I can't see 
how it makes sense to use `role="row"` attributes on what is essentially a 
table cell, not a row.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17819#discussion_r1662673829

Re: RFR: 8299080: Wrong default value of snippet lang attribute

2024-07-01 Thread Hannes Wallnöfer

On Mon, 1 Jul 2024 15:19:22 GMT, Pavel Rappo  wrote:

>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/SnippetTaglet.java
>>  line 498:
>> 
>>> 496: return null;
>>> 497: }
>>> 498: return (lastPeriod == fileName.length() - 1) ? null : 
>>> fileName.substring(lastPeriod + 1);
>> 
>> Some files, like `.gitignore`, only has suffixes, yet they are valid 
>> languages. What do you think?
>
> Sure, that's why there's `<= 0` and not `< 0` one line above that:
> 
> int lastPeriod = fileName.lastIndexOf('.');
> if (lastPeriod <= 0) {
> return null;
> 
> So, if `fileName` starts with `.`, the extension is null.

I think Chen's point was that it should not be `null`, to which I agree, 
although it is probably a corner case. 

One thing I would consider is converting the result or this method to lower 
case, since that is how the language attribute value is usually represented.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19971#discussion_r1661285093

Integrated: 8334241: Adjust API docs side bar dimensions

2024-06-26 Thread Hannes Wallnöfer

On Fri, 14 Jun 2024 16:35:54 GMT, Hannes Wallnöfer  wrote:

> Please review a change to slightly adjust the proportion between the side bar 
> and the main content area in API documentation pages, making the side bar a 
> bit slimmer and leaving more space for the main content area. The ratio 
> between side bar and main content used to be 1 : 2.6, it is now 1 : 3. 
> 
> Updated API documentation[ can be viewed 
> here](https://cr.openjdk.org/~hannesw/8334241/api.00/java.base/java/lang/Object.html).

This pull request has now been integrated.

Changeset: 741a0f39
Author:Hannes Wallnöfer 
URL:   
https://git.openjdk.org/jdk/commit/741a0f39dd1fffc1caaa8d69bfe3662dad830452
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod

8334241: Adjust API docs side bar dimensions

Reviewed-by: jjg

-

PR: https://git.openjdk.org/jdk/pull/19726

Re: RFR: 8325690: The scrollable element with non-interactive content is not tabbable [v38]

2024-06-17 Thread Hannes Wallnöfer

On Fri, 14 Jun 2024 12:50:44 GMT, psoujany  wrote:

>> The scrollable element `` with non-interactive content is not tabbable. 
>> Grid columns in the javadoc stylesheet has overflow: auto, which is failing 
>> Accessibility checks.
>> https://bugs.openjdk.org/browse/JDK-8325690
>
> psoujany has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains 42 additional commits since 
> the last revision:
> 
>  - Merge branch 'openjdk:master' into remove-overflow
>  - Add table role to summary tables
>  - Add table role to summary tables
>  - Add table role to summary tables
>  - Add table role to summary tables
>  - Add grid role to tables
>  - Add grid role to tables
>  - Merge branch 'remove-overflow' of github.com:psoujany/jdk into 
> remove-overflow
>  - Merge branch 'openjdk:master' into remove-overflow
>  - Add grid role to tables
>  - ... and 32 more: https://git.openjdk.org/jdk/compare/a6fa4c6a...8748de2a

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Table.java
 line 341:

> 339: cell.addStyle(tabClass);
> 340: if (!matchFound) {
> 341: cell.put(HtmlAttr.ROLE, "row")

We now use [CSS Grid 
layout](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_grid_layout) for 
tables. Unfortunately, this implies that we do not have HTML elements 
representing rows. Tables are represented by an unstructured stream of table 
cells. This means that 
[`role="cell"`](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/cell_role)
 would probably be the right attribute to use here.

Once 
[Subgrid](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Grid_Layout/Subgrid)
 is well enough supported in browsers (which may be soon), we will be able to 
introduce elements representing table rows.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17819#discussion_r1643053878

RFR: 8334241: Adjust API docs side bar dimensions

2024-06-14 Thread Hannes Wallnöfer

Please review a change to slightly adjust the proportion between the side bar 
and the main content area in API documentation pages, making the side bar a bit 
slimmer and leaving more space for the main content area. The ratio between 
side bar and main content used to be 1 : 2.6, it is now 1 : 3. 

Updated API documentation[ can be viewed 
here](https://cr.openjdk.org/~hannesw/8334241/api.00/java.base/java/lang/Object.html).

-

Commit messages:
 - JDK-8334241: Adjust API docs side bar dimensions

Changes: https://git.openjdk.org/jdk/pull/19726/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19726&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8334241
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/19726.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19726/head:pull/19726

PR: https://git.openjdk.org/jdk/pull/19726

Re: RFR: 8322708: Global HTML attributes are not allowed [v3]

2024-06-14 Thread Hannes Wallnöfer

On Wed, 12 Jun 2024 09:02:20 GMT, Hannes Wallnöfer  wrote:

>> I suggest one of
>> 
>> 1. Add a `boolean global;` member to the enum, and provide a second 
>> constructor that allows that boolean member to be set true.   For example,
>> 
>> WIDTH,
>> STYLE(true),
>> 
>> 
>> 2. Make the `Set` be static and init it in a static initializer.
>> See the `javac` `Source` and `Target` classes for examples of this technique.
>
> Wouldn't the easiest solution be to add a boolean `global`/`isGlobal` field 
> and getter to `Attr`?
> 
> That would give use some more opportunities to simplify the code: We could 
> get rid of the `GLOBAL_ATTRS` map here *and* avoid putting global attributes 
> in the the `attr` map of each `HtmlTag` instance by simply adding a shortcut 
> to return `AttrKind.OK` in `getAttrKind(Name)` if the given attribute is 
> global.

BTW, this should also be done by global attributes that are already respected, 
such as `id`, `class`, and the `aria-*` accessibility attributes.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1636095371

Re: RFR: 8322708: Global HTML attributes are not allowed [v3]

2024-06-14 Thread Hannes Wallnöfer

On Tue, 11 Jun 2024 21:58:46 GMT, Jonathan Gibbons  wrote:

>>> If we are ordering the entries, we can use comparable to check that an attr 
>>> is greater than the start of the global attr, something like
>>> 
>>> ```java
>>> private static boolean isGlobalAttr(Attr value) {
>>> return value.compareTo(Attr.ACCESSKEY) >= 0;
>>> }
>>> ```
>> 
>> That would also work, yes. I still avoid relying on the order of enum 
>> constants; but it's me.
>
> I suggest one of
> 
> 1. Add a `boolean global;` member to the enum, and provide a second 
> constructor that allows that boolean member to be set true.   For example,
> 
> WIDTH,
> STYLE(true),
> 
> 
> 2. Make the `Set` be static and init it in a static initializer.
> See the `javac` `Source` and `Target` classes for examples of this technique.

Wouldn't the easiest solution be to add a boolean `global`/`isGlobal` field and 
getter to `Attr`?

That would give use some more opportunities to simplify the code: We could get 
rid of the `GLOBAL_ATTRS` map here *and* avoid putting global attributes in the 
the `attr` map of each `HtmlTag` instance by simply adding a shortcut to return 
`AttrKind.OK` in `getAttrKind(Name)` if the given attribute is global.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1636093592

Integrated: 8331947: Preview creates checkbox for JEP-less preview feature

2024-06-05 Thread Hannes Wallnöfer

On Wed, 22 May 2024 08:39:12 GMT, Hannes Wallnöfer  wrote:

> Please review a simple patch to exclude preview visitor classes meant to 
> support future preview features from the Preview API page. 
> 
> The test adds an sample element annotated with the new 
> `PreviewFeature.Feature.LANGUAGE_MODEL` constant (which does not have a 
> `@JEP` annotation) to make sure it is not listed in the Preview API page. The 
> test itself does not have to be modified, as it would fail without the change 
> in `PreviewAPIListBuilder.java`.

This pull request has now been integrated.

Changeset: 765ad0e4
Author:Hannes Wallnöfer 
URL:   
https://git.openjdk.org/jdk/commit/765ad0e40bc522de4b2821ccc60b9139faf7376f
Stats: 97 lines in 8 files changed: 46 ins; 11 del; 40 mod

8331947: Preview creates checkbox for JEP-less preview feature

Reviewed-by: liach, prappo

-

PR: https://git.openjdk.org/jdk/pull/19344

Re: RFR: 8331947: Preview creates checkbox for JEP-less preview feature [v4]

2024-06-05 Thread Hannes Wallnöfer

On Wed, 5 Jun 2024 11:04:06 GMT, Pavel Rappo  wrote:

>> Hannes Wallnöfer has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Address review feedback
>
> test/langtools/jdk/javadoc/doclet/testPreview/TestPreview.java line 161:
> 
>> 159: """);
>> 160: 
>> 161: // 8331947: Support preview features without JEP should not be 
>> included in Preview API page
> 
> Trivial: here it is phrased as "Support preview features", while in the 
> actual test source file it is "Preview support feature".

I agree the terminology in comments is somewhat inconsistent. I don't think 
there is an "official" term for these preview features. [In the 
CSR](https://bugs.openjdk.org/browse/JDK-8329634) they are summarized as 
follows: "Add a set of persistent preview visitors to support new language 
changes in development." 

I think the essential property for our purpose is the "without JEP". I added 
the "support" adjective to make their purpose clearer, but I don't think it's 
essential.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19344#discussion_r1627680852

Integrated: 8332039: Cannot invoke "com.sun.source.util.DocTreePath.getTreePath()" because "path" is null

2024-06-04 Thread Hannes Wallnöfer

On Thu, 23 May 2024 10:39:52 GMT, Hannes Wallnöfer  wrote:

> Please review a patch to fix a NPE thrown when a `@since` tag inherited by a 
> nested class contains a nested inline tag. The solution is to make 
> `CommentHelper.getDocTreePath(DocTree)` able to handle block tags inherited 
> by nested classes.

This pull request has now been integrated.

Changeset: a706e35b
Author:Hannes Wallnöfer 
URL:   
https://git.openjdk.org/jdk/commit/a706e35b12addff987b489059be8f240c60fae75
Stats: 68 lines in 2 files changed: 49 ins; 3 del; 16 mod

8332039: Cannot invoke "com.sun.source.util.DocTreePath.getTreePath()" because 
"path" is null

Reviewed-by: jjg

-

PR: https://git.openjdk.org/jdk/pull/19363

Re: RFR: 8332039: Cannot invoke "com.sun.source.util.DocTreePath.getTreePath()" because "path" is null

2024-06-04 Thread Hannes Wallnöfer

On Thu, 23 May 2024 10:39:52 GMT, Hannes Wallnöfer  wrote:

> Please review a patch to fix a NPE thrown when a `@since` tag inherited by a 
> nested class contains a nested inline tag. The solution is to make 
> `CommentHelper.getDocTreePath(DocTree)` able to handle block tags inherited 
> by nested classes.

I filed a follow-up issue to refactor the code for inherited and derived doc 
comments:

https://bugs.openjdk.org/browse/JDK-8333557

-

PR Comment: https://git.openjdk.org/jdk/pull/19363#issuecomment-2148040277

Re: RFR: 8325690: The scrollable element with non-interactive content is not tabbable [v29]

2024-06-04 Thread Hannes Wallnöfer

On Mon, 3 Jun 2024 12:33:31 GMT, psoujany  wrote:

>> The scrollable element `` with non-interactive content is not tabbable. 
>> Grid columns in the javadoc stylesheet has overflow: auto, which is failing 
>> Accessibility checks.
>> https://bugs.openjdk.org/browse/JDK-8325690
>
> psoujany has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add tabindex to tabbable elements

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Table.java
 line 334:

> 332: cell.addStyle(rowStyle);
> 333: if (!matchFound) {
> 334: cell.put(HtmlAttr.ROLE, "tablist")

Looking into this further, I don't think the `role="tablist"` attribute is 
correct here. According to the [MDN documentation], a `tablist` is defined as a 
container for a set of `tabs`, which does not apply for table cells.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17819#discussion_r1625750412

Re: RFR: 8332039: Cannot invoke "com.sun.source.util.DocTreePath.getTreePath()" because "path" is null

2024-06-03 Thread Hannes Wallnöfer

On Fri, 31 May 2024 20:58:31 GMT, Jonathan Gibbons  wrote:

> > To clarify, I'm not too concerned with how we call an action whereby a 
> > nested class gets its `@since` tag from the enclosing class; but yes, 
> > "inheritance" might not be ideal. What I'm concerned with is the fact that 
> > the logic originally provided in 
> > [`getDefaultBlockTags`](https://github.com/openjdk/jdk/blob/6ee8407758c92d32e18642b0758d2d5c71ad09f5/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/SimpleTaglet.java#L141-L147)
> >  is now duplicated in `getInheritedDocTreePath`.
> > Like I said, while it's bad, the new duplication merely replicates the one 
> > we already have between `InheritableTaglet.inherit` and 
> > `getInheritedDocTreePath`.
> 
> If we can't fix this in the next few days, we should file a cleanup issue to 
> resolve this duplication.

I don't quite see the problem with this, or how to improve it substantially 
(beyond finding better terminology). We have different ways of obtaining doc 
comment fragments from related elements. The major and well-established one is 
along the lines of Java type inheritance, and a minor/new one along 
enclosing/enclosed classes. Since `CommentHelper` provides a means to reverse 
lookup `DocTree` -> `DocTreePath`, it necessarily needs to know about all of 
these mechanisms. The code for the enclosing/enclosed classes is actually 
trivial, so I don't see how this could be simplified further. The code for 
finding doc comments along type inheritance is more complex, but it already 
delegates to the `DocFinder` class, which is also used for the primary lookup 
for inherited doc comments. 

I certainly don't think the existing/proposed code is perfect, but I don't see 
huge potential for improvement. Of course I'm open to learn and better 
understand the issues, and how to solve them.

-

PR Comment: https://git.openjdk.org/jdk/pull/19363#issuecomment-2145104410

Re: RFR: 8325690: The scrollable element with non-interactive content is not tabbable [v23]

On Mon, 27 May 2024 09:50:38 GMT, psoujany  wrote:

>> The scrollable element `` with non-interactive content is not tabbable. 
>> Grid columns in the javadoc stylesheet has overflow: auto, which is failing 
>> Accessibility checks.
>> https://bugs.openjdk.org/browse/JDK-8325690
>
> psoujany has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Add tab order to plain text
>  - Add tab order to plain text

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Table.java
 line 335:

> 333: }
> 334: int colIndex = 0;
> 335: String regex = 
> "<(?:a|area|button|input|object|select|textarea)\\b";

The only element that will likely occur in a API description is `a`. Is it 
actually the case that a contained link will make the table cell tabbable and 
scrollable via keyboard?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17819#discussion_r1622503035

Re: RFR: 8325690: The scrollable element with non-interactive content is not tabbable [v23]

On Mon, 27 May 2024 09:50:38 GMT, psoujany  wrote:

>> The scrollable element `` with non-interactive content is not tabbable. 
>> Grid columns in the javadoc stylesheet has overflow: auto, which is failing 
>> Accessibility checks.
>> https://bugs.openjdk.org/browse/JDK-8325690
>
> psoujany has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Add tab order to plain text
>  - Add tab order to plain text

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Table.java
 line 349:

> 347:  * For more information on the `widget_tabbable_single` 
> rule, please refer to the following documentation:
> 348:  *  href="https://able.ibm.com/rules/archives/latest/doc/en-US/widget_tabbable_single.html#%7B%22message%22%3A%22Component%20with%20%5C%22tablist%5C%22%20role%20has%20more%20than%20one%20tabbable%20element%22%2C%22snippet%22%3A%22%3Cdiv%20tabindex%3D%5C%220%5C%22%20role%3D%5C%22tablist%5C%22%20class%3D%5C%22col-second%20odd-row-color%20method-summary-table%20method-summary-table-tab2%20method-summary-table-tab3%5C%22%3E%22%2C%22value%22%3A%5B%22VIOLATION%22%2C%22POTENTIAL%22%5D%2C%22reasonId%22%3A%22fail_multiple_tabbable%22%2C%22ruleId%22%3A%22widget_tabbable_single%22%7D";>widget_tabbable_single
> 349:  */

I think this new comment is a bit excessive. Can it be boiled down to a single 
sentence, maybe one or two lines? Also, there's no need to use `/**` doc 
comment style here. For example (but feel free to improve):

// Only make element tabbable if it doesn't contain tabbable content.

Regarding the link, I don't think the content after the `#` is necessary. I 
would prefer the following form:

// 
https://able.ibm.com/rules/archives/latest/doc/en-US/widget_tabbable_single.html

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17819#discussion_r1622499481

Integrated: 8331579: Reference to primitive type fails without error or warning

On Tue, 28 May 2024 12:28:53 GMT, Hannes Wallnöfer  wrote:

> Please review a simple fix to make DocLint report an error when linking to 
> non-declared types in `{@link}` or `@see` tags. This has been implemented 
> when the Standard Doclet is running without DocLint in 
> [JDK-8284030](https://bugs.openjdk.org/browse/JDK-8284030), but needs to be 
> implemented in DocLint as well. It was necessary to limit the behaviour to 
> `{@link}` or `@see`tags since references to non-declared types are allowed 
> within the `@serialField` taglet.

This pull request has now been integrated.

Changeset: 1e04ee6d
Author:Hannes Wallnöfer 
URL:   
https://git.openjdk.org/jdk/commit/1e04ee6d57d5fe84e1d202b16e8d13dc13c002ff
Stats: 125 lines in 2 files changed: 123 ins; 0 del; 2 mod

8331579: Reference to primitive type fails without error or warning

Reviewed-by: jjg, prappo

-

PR: https://git.openjdk.org/jdk/pull/19423

Re: RFR: 8329537: Nested and enclosing classes should be linked separately in breadcrumb navigation [v2]

> Please review a patch to change the breadcrumb sub-navigation in API docs to 
> display nested classes as separate links. 
> 
> The change itself is simple, I replaced the `getBreadCrumbLink` method in 
> `HtmlDocletWriter` which returned a link for a single element with a new 
> `addBreadCrumbLinks` method in `Navigation` which recursively adds all 
> breadcrumb links to a list. Moving the code from `HtmlDocletWriter` to 
> `Navigation` allowed me to remove a lot of code because it is no longer 
> necessary to override the `getNavBar` method in a lot of writer subclasses.
> 
> I compared the JDI API documentation with and without this patch and there 
> were no changes except for the breadcrumb links for nested classes.

Hannes Wallnöfer has updated the pull request incrementally with one additional 
commit since the last revision:

  Address review feedback, simplify comment

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19466/files
  - new: https://git.openjdk.org/jdk/pull/19466/files/1468be35..ea959c59

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=19466&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19466&range=00-01

  Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/19466.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19466/head:pull/19466

PR: https://git.openjdk.org/jdk/pull/19466

Integrated: 8329537: Nested and enclosing classes should be linked separately in breadcrumb navigation

On Wed, 29 May 2024 19:39:14 GMT, Hannes Wallnöfer  wrote:

> Please review a patch to change the breadcrumb sub-navigation in API docs to 
> display nested classes as separate links. 
> 
> The change itself is simple, I replaced the `getBreadCrumbLink` method in 
> `HtmlDocletWriter` which returned a link for a single element with a new 
> `addBreadCrumbLinks` method in `Navigation` which recursively adds all 
> breadcrumb links to a list. Moving the code from `HtmlDocletWriter` to 
> `Navigation` allowed me to remove a lot of code because it is no longer 
> necessary to override the `getNavBar` method in a lot of writer subclasses.
> 
> I compared the JDI API documentation with and without this patch and there 
> were no changes except for the breadcrumb links for nested classes.

This pull request has now been integrated.

Changeset: e930bc1f
Author:Hannes Wallnöfer 
URL:   
https://git.openjdk.org/jdk/commit/e930bc1f0f2f8a4d03bfe7a8f4420335a8a2
Stats: 196 lines in 10 files changed: 66 ins; 128 del; 2 mod

8329537: Nested and enclosing classes should be linked separately in breadcrumb 
navigation

Reviewed-by: jjg

-

PR: https://git.openjdk.org/jdk/pull/19466

Re: RFR: 8331947: Preview creates checkbox for JEP-less preview feature [v4]

On Fri, 31 May 2024 10:01:17 GMT, Hannes Wallnöfer  wrote:

>> Please review a simple patch to exclude preview visitor classes meant to 
>> support future preview features from the Preview API page. 
>> 
>> The test adds an sample element annotated with the new 
>> `PreviewFeature.Feature.LANGUAGE_MODEL` constant (which does not have a 
>> `@JEP` annotation) to make sure it is not listed in the Preview API page. 
>> The test itself does not have to be modified, as it would fail without the 
>> change in `PreviewAPIListBuilder.java`.
>
> Hannes Wallnöfer has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Address review feedback

Thanks @liach, I updated the PR.

-

PR Comment: https://git.openjdk.org/jdk/pull/19344#issuecomment-2141680454

Re: RFR: 8331947: Preview creates checkbox for JEP-less preview feature [v4]

> Please review a simple patch to exclude preview visitor classes meant to 
> support future preview features from the Preview API page. 
> 
> The test adds an sample element annotated with the new 
> `PreviewFeature.Feature.LANGUAGE_MODEL` constant (which does not have a 
> `@JEP` annotation) to make sure it is not listed in the Preview API page. The 
> test itself does not have to be modified, as it would fail without the change 
> in `PreviewAPIListBuilder.java`.

Hannes Wallnöfer has updated the pull request incrementally with one additional 
commit since the last revision:

  Address review feedback

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19344/files
  - new: https://git.openjdk.org/jdk/pull/19344/files/89a201db..134f274c

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=19344&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19344&range=02-03

  Stats: 9 lines in 2 files changed: 5 ins; 0 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/19344.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19344/head:pull/19344

PR: https://git.openjdk.org/jdk/pull/19344

RFR: 8329537: Nested and enclosing classes should be linked separately in breadcrumb navigation

2024-05-29 Thread Hannes Wallnöfer

Please review a patch to change the breadcrumb sub-navigation in API docs to 
display nested classes as separate links. 

The change itself is simple, I replaced the `getBreadCrumbLink` method in 
`HtmlDocletWriter` which returned a link for a single element with a new 
`addBreadCrumbLinks` method in `Navigation` which recursively adds all 
breadcrumb links to a list. Moving the code from `HtmlDocletWriter` to 
`Navigation` allowed me to remove a lot of code because it is no longer 
necessary to override the `getNavBar` method in a lot of writer subclasses.

I compared the JDI API documentation with and without this patch and there were 
no changes except for the breadcrumb links for nested classes.

-

Commit messages:
 - JDK-8329537: Nested and enclosing classes should be linked separately in 
breadcrumb navigation

Changes: https://git.openjdk.org/jdk/pull/19466/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19466&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8329537
  Stats: 196 lines in 10 files changed: 66 ins; 128 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/19466.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19466/head:pull/19466

PR: https://git.openjdk.org/jdk/pull/19466

RFR: 8331579: Reference to primitive type fails without error or warning

2024-05-28 Thread Hannes Wallnöfer

Please review a simple fix to make DocLint report an error when linking to 
non-declared types in `{@link}` or `@see` tags. This has been implemented when 
the Standard Doclet is running without DocLint in 
[JDK-8284030](https://bugs.openjdk.org/browse/JDK-8284030), but needs to be 
implemented in DocLint as well. It was necessary to limit the behaviour to 
`{@link}` or `@see`tags since references to non-declared types are allowed 
within the `@serialField` taglet.

-

Commit messages:
 - JDK-8331579: Reference to primitive type fails without error or warning

Changes: https://git.openjdk.org/jdk/pull/19423/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19423&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8331579
  Stats: 125 lines in 2 files changed: 123 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/19423.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19423/head:pull/19423

PR: https://git.openjdk.org/jdk/pull/19423

Re: RFR: 8331873: Improve/expand info in `New API In` on Help page

2024-05-24 Thread Hannes Wallnöfer

On Mon, 13 May 2024 23:08:51 GMT, Jonathan Gibbons  wrote:

> Please review a relatively simple update to the generated Help page, as part 
> of the ongoing campaign to improve the documentation around the overall use 
> of `@since` tags.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties
 line 381:

> 379: # The title for the Javadoc Search Specification
> 380: doclet.help.search.spec.title=Javadoc Search Specification
> 381: doclet.help.releases.head=Release Details

The subtitle "Release Details" surprised me, because it seems to suggest 
"details about releases", while it is more like "release information (contained 
in API details)".

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties
 line 389:

> 387: include the release in which the member was introduced, in those 
> situations \
> 388: where the member was added after the initial introduction of the \
> 389: enclosing class or interface.

I think "normally" is confusing here, because a member being added in a later 
release is not necessary the "normal" case. How about reversing the sentence to 
make it immediately clear what we're talking about?

> In those situations where a member was added after the initial introduction 
> of the enclosing class or interface, the detail of the member includes the 
> release in which it was introduced.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties
 line 392:

> 390: doclet.help.releases.body.refer=\
> 391: Some summary pages allow you to filter the contents of the page 
> according to \
> 392: the release in which the declaration was introduced or deprecated.

We already have help sections for New and Deprecated API, and the paragraph is 
somewhat vague. I think we should at least mention the summary pages by name 
("New API", "Deprecated API"). Maybe link to the pages or help sections?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19222#discussion_r1613689324
PR Review Comment: https://git.openjdk.org/jdk/pull/19222#discussion_r1613686145
PR Review Comment: https://git.openjdk.org/jdk/pull/19222#discussion_r1613678814

Re: RFR: 8325690: The scrollable element with non-interactive content is not tabbable [v20]

2024-05-24 Thread Hannes Wallnöfer

On Tue, 30 Apr 2024 10:52:23 GMT, psoujany  wrote:

>> The scrollable element `` with non-interactive content is not tabbable. 
>> Grid columns in the javadoc stylesheet has overflow: auto, which is failing 
>> Accessibility checks.
>> https://bugs.openjdk.org/browse/JDK-8325690
>
> psoujany has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove trailing whitespaces

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Table.java
 line 339:

> 337: for (Content c : contents) {
> 338: HtmlStyle cellStyle = columnStyles.get(colIndex);
> 339: // Add tab order to only plain text to avoid 
> widget_tabbable_single(potential violation)

`widget_tabbable_single` sounds like a programmatic name. Can we provide a 
better explanation of what this rule means, maybe also adding a link? [This 
piece of code][tabbable-single] seems to provide an explanation, but I'm not 
sure what it means in the current context.

[tabbable-single]: 
https://github.com/IBMa/equal-access/blob/7954dcdc5f1b5536057f783abcae1f7521265200/karma-accessibility-checker/test/client/baseline/JSONObjectStructureVerification.html.json#L2810-L2811

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17819#discussion_r1613622805

Re: RFR: 8331947: Preview creates checkbox for JEP-less preview feature [v3]

On Thu, 23 May 2024 13:59:19 GMT, Hannes Wallnöfer  wrote:

>> Please review a simple patch to exclude preview visitor classes meant to 
>> support future preview features from the Preview API page. 
>> 
>> The test adds an sample element annotated with the new 
>> `PreviewFeature.Feature.LANGUAGE_MODEL` constant (which does not have a 
>> `@JEP` annotation) to make sure it is not listed in the Preview API page. 
>> The test itself does not have to be modified, as it would fail without the 
>> change in `PreviewAPIListBuilder.java`.
>
> Hannes Wallnöfer has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains three commits:
> 
>  - Merge branch 'master' of https://git.openjdk.org/jdk into JDK-8331947
>  - Only include preview features with a JEP
>  - JDK-8331947: Preview creates checkbox for JEP-less preview feature

I updated this PR to exclude elements of preview features that don't have a JEP 
from the Preview API page. 

In order to do this, I changed the way `SummaryAPIListBuilder` determines which 
API elements to include from using a `Predicate` passed to the constructor to 
using a new abstract `belongsToSummary(Element)` method. The reason for this is 
that the predicate had to be passed to the builder's super-constructor, which 
made it impossible to use an instance method of the current builder. However, 
since determining whether a preview API element has a JEP is somewhat involved 
(retrieving annotation elements of an enum constant used as annotation 
element), we only want to do this once, and ideally the first time we look at 
an element. Being able to use an ordinary instance method was the simplest 
solution to this problem.

I also changed the workaround method that retrieves the JEP info to return a 
`Map` instead of the the raw `javax.lang.model.element` types, 
which makes it easier to use.

-

PR Comment: https://git.openjdk.org/jdk/pull/19344#issuecomment-2127244561

Re: RFR: 8331947: Preview creates checkbox for JEP-less preview feature [v3]

> Please review a simple patch to exclude preview visitor classes meant to 
> support future preview features from the Preview API page. 
> 
> The test adds an sample element annotated with the new 
> `PreviewFeature.Feature.LANGUAGE_MODEL` constant (which does not have a 
> `@JEP` annotation) to make sure it is not listed in the Preview API page. The 
> test itself does not have to be modified, as it would fail without the change 
> in `PreviewAPIListBuilder.java`.

Hannes Wallnöfer has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains three commits:

 - Merge branch 'master' of https://git.openjdk.org/jdk into JDK-8331947
 - Only include preview features with a JEP
 - JDK-8331947: Preview creates checkbox for JEP-less preview feature

-

Changes: https://git.openjdk.org/jdk/pull/19344/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19344&range=02
  Stats: 91 lines in 8 files changed: 41 ins; 11 del; 39 mod
  Patch: https://git.openjdk.org/jdk/pull/19344.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19344/head:pull/19344

PR: https://git.openjdk.org/jdk/pull/19344

Re: RFR: 8331947: Preview creates checkbox for JEP-less preview feature [v2]

> Please review a simple patch to exclude preview visitor classes meant to 
> support future preview features from the Preview API page. 
> 
> The test adds an sample element annotated with the new 
> `PreviewFeature.Feature.LANGUAGE_MODEL` constant (which does not have a 
> `@JEP` annotation) to make sure it is not listed in the Preview API page. The 
> test itself does not have to be modified, as it would fail without the change 
> in `PreviewAPIListBuilder.java`.

Hannes Wallnöfer has updated the pull request incrementally with one additional 
commit since the last revision:

  Only include preview features with a JEP

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19344/files
  - new: https://git.openjdk.org/jdk/pull/19344/files/5e7cd0de..c3a92a2f

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=19344&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19344&range=00-01

  Stats: 89 lines in 7 files changed: 34 ins; 13 del; 42 mod
  Patch: https://git.openjdk.org/jdk/pull/19344.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19344/head:pull/19344

PR: https://git.openjdk.org/jdk/pull/19344

RFR: 8332039: Cannot invoke "com.sun.source.util.DocTreePath.getTreePath()" because "path" is null

Please review a patch to fix a NPE thrown when a `@since` tag inherited by a 
nested class contains a nested inline tag. The solution is to make 
`CommentHelper.getDocTreePath(DocTree)` able to handle block tags inherited by 
nested classes.

-

Commit messages:
 - JDK-8332039: Cannot invoke "com.sun.source.util.DocTreePath.getTreePath()" 
because "path" is null

Changes: https://git.openjdk.org/jdk/pull/19363/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19363&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8332039
  Stats: 68 lines in 2 files changed: 49 ins; 3 del; 16 mod
  Patch: https://git.openjdk.org/jdk/pull/19363.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19363/head:pull/19363

PR: https://git.openjdk.org/jdk/pull/19363

Re: RFR: 8331947: Preview creates checkbox for JEP-less preview feature

2024-05-22 Thread Hannes Wallnöfer

On Wed, 22 May 2024 08:39:12 GMT, Hannes Wallnöfer  wrote:

> Please review a simple patch to exclude preview visitor classes meant to 
> support future preview features from the Preview API page. 
> 
> The test adds an sample element annotated with the new 
> `PreviewFeature.Feature.LANGUAGE_MODEL` constant (which does not have a 
> `@JEP` annotation) to make sure it is not listed in the Preview API page. The 
> test itself does not have to be modified, as it would fail without the change 
> in `PreviewAPIListBuilder.java`.

Note: this patch only removes the checkbox labeled "0: (Preview)" from the top 
of the Preview API page, but the preview visitor classes themselves are still 
contained in the page. I'm not sure they need to be, so this PR may see further 
updates.

-

PR Comment: https://git.openjdk.org/jdk/pull/19344#issuecomment-2124777673

Re: RFR: 8331947: Preview creates checkbox for JEP-less preview feature

2024-05-22 Thread Hannes Wallnöfer

On Wed, 22 May 2024 11:26:02 GMT, Chen Liang  wrote:

>> Please review a simple patch to exclude preview visitor classes meant to 
>> support future preview features from the Preview API page. 
>> 
>> The test adds an sample element annotated with the new 
>> `PreviewFeature.Feature.LANGUAGE_MODEL` constant (which does not have a 
>> `@JEP` annotation) to make sure it is not listed in the Preview API page. 
>> The test itself does not have to be modified, as it would fail without the 
>> change in `PreviewAPIListBuilder.java`.
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/PreviewAPIListBuilder.java
>  line 91:
> 
>> 89: if (jep.number == 0) {
>> 90: // Remove preview support features without a valid JEP
>> 91: jeps.remove(feature);
> 
> This feels like we are adding and removing a dummy jep to and from the map 
> every time we encounter the language model feature. Can we just bind a dummy 
> jep to that feature to avoid repeated processing, and only filter at the 
> getter methods?

I agree it's not ideal. I think the best thing is to split up the insertion 
into separate steps: First check if the key is contained in the map, retrieve 
the JEP info if it isn't, and only then decide whether to create a JEP record.

But I found another problem with this PR, so there may be some further changes 
coming.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19344#discussion_r1609881611

RFR: 8331947: Preview creates checkbox for JEP-less preview feature

2024-05-22 Thread Hannes Wallnöfer

Please review a simple patch to exclude preview visitor classes meant to 
support future preview features from the Preview API page. 

The test adds an sample element annotated with the new 
`PreviewFeature.Feature.LANGUAGE_MODEL` constant (which does not have a `@JEP` 
annotation) to make sure it is not listed in the Preview API page. The test 
itself does not have to be modified, as it would fail without the change in 
`PreviewAPIListBuilder.java`.

-

Commit messages:
 - JDK-8331947: Preview creates checkbox for JEP-less preview feature

Changes: https://git.openjdk.org/jdk/pull/19344/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19344&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8331947
  Stats: 13 lines in 3 files changed: 9 ins; 0 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/19344.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19344/head:pull/19344

PR: https://git.openjdk.org/jdk/pull/19344

Integrated: 8332239: Improve CSS for block tags

On Thu, 16 May 2024 10:56:26 GMT, Hannes Wallnöfer  wrote:

> Please review a change to improve the layout of definition lists used to 
> display block tags:
> 
>  - Add indentation to the `` elements used for block tag details
>  - Set the margin of lists within block tag details so they do not appear as 
> nested lists, except for lists with CSS classes `tag-list` or 
> `tag-list-long`, which are used for block tags containing lists, such as 
> `@see`. 
> 
> Before/after comparison (contains `java.base` and `java.compiler` modules):
> https://cr.openjdk.org/~hannesw/8332239/api.00/index.html
> https://cr.openjdk.org/~hannesw/8332239/api.01/index.html
> 
> Comparison for block tag layout:
> https://cr.openjdk.org/~hannesw/8332239/api.00/java.base/java/lang/Object.html#hashCode()
> https://cr.openjdk.org/~hannesw/8332239/api.01/java.base/java/lang/Object.html#hashCode()
> 
> Example of very long block tag details containing a list:
> https://cr.openjdk.org/~hannesw/8332239/api.00/java.compiler/javax/lang/model/util/package-summary.html
> https://cr.openjdk.org/~hannesw/8332239/api.01/java.compiler/javax/lang/model/util/package-summary.html
> 
> Note that the indentation also applies to the definition lists at the top of 
> class/interface documentation as they use the same markup as block tags:
> https://cr.openjdk.org/~hannesw/8332239/api.00/java.base/java/lang/Exception.html
> https://cr.openjdk.org/~hannesw/8332239/api.01/java.base/java/lang/Exception.html
> 
>  This was not an intended change but might or might not be desirable. It 
> could be avoided with some additional CSS changes.

This pull request has now been integrated.

Changeset: 44bdf996
Author:Hannes Wallnöfer 
URL:   
https://git.openjdk.org/jdk/commit/44bdf9964eb2dd0eb4034576e1f903a27c410286
Stats: 8 lines in 1 file changed: 5 ins; 0 del; 3 mod

8332239: Improve CSS for block tags

Reviewed-by: prappo, liach, jjg

-

PR: https://git.openjdk.org/jdk/pull/19264

Integrated: 8317621: --add-script should support JavaScript modules

On Fri, 29 Mar 2024 09:39:18 GMT, Hannes Wallnöfer  wrote:

> Please review an enhancement to detect [JavaScript 
> modules](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules)
>  when added to `javadoc` with the `--add-script` option, which require a 
> different `type` attribute in the HTML `

Integrated: 8329617: Update stylesheet for specs and tool documentation

On Tue, 9 Apr 2024 13:06:28 GMT, Hannes Wallnöfer  wrote:

> Please review an update to the `jdk-default.css` stylesheet used for 
> specifications and tool guides. The original purpose was to make use of the 
> Dejavu web fonts provided by the API docs and to update the navigation bar to 
> match the one in the API docs. However, the updates include some other fixes 
> and improvements also described below.
> 
>  - The change to use the DejaVu web fonts consists only of the `@import` 
> statement in line 16 as the stylesheet already used DejaVu web fonts as first 
> choice in its `font-family` rules.
>  - The changes to make the navigation bar match the one in the API docs are 
> mostly located at the end of the file (beyond line 160). However, this also 
> includes setting the `margin` property to '0' in the `body` element and 
> adding a `margin` in the `main` and `footer` elements instead. 
>  - To set the horizontal margin for page content elements outside the `main` 
> element which occur in some pages, a margin is set explicitly on those 
> elements in lines 48-50. While this is a bit awkward, I think it's still 
> better than working with negative margins in the header to offset the margin 
> in the `body` element.
>  - Most of the remaining changes (lines 53-110) are changes are to redefine 
> the styles in simpler terms, such as leaving out declarations that are equal 
> to browser defaults, and removing the units from `0`-length values.
> 
> The changes are intended to preserve the layout of the pages, including the 
> body font size which is slightly different from the one used in API docs 
> (`10pt` vs `14px`). I can provide before/after snapshots of the rendered 
> documentation if desired.

This pull request has now been integrated.

Changeset: 4eb1eaf0
Author:Hannes Wallnöfer 
URL:   
https://git.openjdk.org/jdk/commit/4eb1eaf04477b9a8947a57655cf36380b5b88b5c
Stats: 73 lines in 1 file changed: 27 ins; 19 del; 27 mod

8329617: Update stylesheet for specs and tool documentation

Reviewed-by: jjg

-

PR: https://git.openjdk.org/jdk/pull/18694

Re: RFR: 8298405: Implement JEP 467: Markdown Documentation Comments [v70]

On Thu, 16 May 2024 18:17:31 GMT, Jonathan Gibbons  wrote:

>> Please review a patch to add support for Markdown syntax in documentation 
>> comments, as described in the associated JEP.
>> 
>> Notable features:
>> 
>> * support for `///` documentation comments in `JavaTokenizer`
>> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` 
>> library
>> * updates to `DocCommentParser` to treat `///` comments as Markdown
>> * updates to the standard doclet to render Markdown comments in HTML
>
> Jonathan Gibbons has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   address review feedback

@jonathan-gibbons I'm extremely impressed with this work, especially given its 
size and the non-trivial problems you had to solve. Kudos also to @pavelrappo 
for relentless reviewing!

-

Marked as reviewed by hannesw (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16388#pullrequestreview-2062926643

Re: RFR: 8298405: Implement JEP 467: Markdown Documentation Comments [v69]

2024-05-16 Thread Hannes Wallnöfer

On Wed, 15 May 2024 21:04:36 GMT, Jonathan Gibbons  wrote:

>> Please review a patch to add support for Markdown syntax in documentation 
>> comments, as described in the associated JEP.
>> 
>> Notable features:
>> 
>> * support for `///` documentation comments in `JavaTokenizer`
>> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` 
>> library
>> * updates to `DocCommentParser` to treat `///` comments as Markdown
>> * updates to the standard doclet to render Markdown comments in HTML
>
> Jonathan Gibbons has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   update tests for dangling doc comments, per review feedback

src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java
 line 276:

> 274: // The following {@code transform} methods invoke {@code 
> transform} on
> 275: // any children that may contain Markdown. If the 
> transformations on
> 276: // the children are all identify transformations (that is the 
> result

Typo: identify -> identity

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
 line 1465:

> 1463: private final HtmlRenderer renderer = HtmlRenderer.builder()
> 1464: .nodeRendererFactory(headingRendererFactory)
> 1465: .extensions(List.of(tablesExtn/*, headingIdsExtn*/))

Is there a reason to keep the commented argument?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1603439954
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1603716675

Re: RFR: 8332239: Improve CSS for block tags

2024-05-16 Thread Hannes Wallnöfer

On Thu, 16 May 2024 11:21:14 GMT, Pavel Rappo  wrote:

>> Please review a change to improve the layout of definition lists used to 
>> display block tags:
>> 
>>  - Add indentation to the `` elements used for block tag details
>>  - Set the margin of lists within block tag details so they do not appear as 
>> nested lists, except for lists with CSS classes `tag-list` or 
>> `tag-list-long`, which are used for block tags containing lists, such as 
>> `@see`. 
>> 
>> Before/after comparison (contains `java.base` and `java.compiler` modules):
>> https://cr.openjdk.org/~hannesw/8332239/api.00/index.html
>> https://cr.openjdk.org/~hannesw/8332239/api.01/index.html
>> 
>> Comparison for block tag layout:
>> https://cr.openjdk.org/~hannesw/8332239/api.00/java.base/java/lang/Object.html#hashCode()
>> https://cr.openjdk.org/~hannesw/8332239/api.01/java.base/java/lang/Object.html#hashCode()
>> 
>> Example of very long block tag details containing a list:
>> https://cr.openjdk.org/~hannesw/8332239/api.00/java.compiler/javax/lang/model/util/package-summary.html
>> https://cr.openjdk.org/~hannesw/8332239/api.01/java.compiler/javax/lang/model/util/package-summary.html
>> 
>> Note that the indentation also applies to the definition lists at the top of 
>> class/interface documentation as they use the same markup as block tags:
>> https://cr.openjdk.org/~hannesw/8332239/api.00/java.base/java/lang/Exception.html
>> https://cr.openjdk.org/~hannesw/8332239/api.01/java.base/java/lang/Exception.html
>> 
>>  This was not an intended change but might or might not be desirable. It 
>> could be avoided with some additional CSS changes.
>
> Looks good in a rendered form. However, that mix of CSS units that we have -- 
> %, px, em -- always looked suspicious to me.

Thanks @pavelrappo.

> However, that mix of CSS units that we have -- %, px, em -- always looked 
> suspicious to me.

There are reasons for using one or the other. `em` is the font size of the 
local element, so it can be used to space things relative to the element's font 
size. `1em` is the default top and bottom margin for top-level lists, so the 
list will be laid out correctly independent of the list's font.

`px` on the other hand is an absolute unit. We use it to set the default font 
size to `14px` as the default `16px` will look too big, and to set up the basic 
layout of the page. In this case, the indentation should be the same regardless 
of font size (which might vary for proportional and monospaced font, for 
example).

-

PR Comment: https://git.openjdk.org/jdk/pull/19264#issuecomment-2115027428

RFR: 8332239: Improve CSS for block tags

2024-05-16 Thread Hannes Wallnöfer

Please review a change to improve the layout of definition lists used to 
display block tags:

 - Add indentation to the `` elements used for block tag details
 - Set the margin of lists within block tag details so they do not appear as 
nested lists, except for lists with CSS classes `tag-list` or `tag-list-long`, 
which are used for block tags containing lists, such as `@see`. 

Before/after comparison (contains `java.base` and `java.compiler` modules):
https://cr.openjdk.org/~hannesw/8332239/api.00/index.html
https://cr.openjdk.org/~hannesw/8332239/api.01/index.html

Comparison for block tag layout:
https://cr.openjdk.org/~hannesw/8332239/api.00/java.base/java/lang/Object.html#hashCode()
https://cr.openjdk.org/~hannesw/8332239/api.01/java.base/java/lang/Object.html#hashCode()

Example of very long block tag details containing a list:
https://cr.openjdk.org/~hannesw/8332239/api.00/java.compiler/javax/lang/model/util/package-summary.html
https://cr.openjdk.org/~hannesw/8332239/api.01/java.compiler/javax/lang/model/util/package-summary.html

Note that the indentation also applies to the definition lists at the top of 
class/interface documentation as they use the same markup as block tags. This 
was not an intended change, but is hard to avoid without deeper changes to the 
markup or CSS:
https://cr.openjdk.org/~hannesw/8332239/api.00/java.base/java/lang/Exception.html
https://cr.openjdk.org/~hannesw/8332239/api.01/java.base/java/lang/Exception.html

-

Commit messages:
 - JDK-8332239: Improve CSS for block tags

Changes: https://git.openjdk.org/jdk/pull/19264/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19264&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8332239
  Stats: 8 lines in 1 file changed: 5 ins; 0 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/19264.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19264/head:pull/19264

PR: https://git.openjdk.org/jdk/pull/19264

Re: RFR: 8317621: --add-script should support JavaScript modules [v2]

2024-05-15 Thread Hannes Wallnöfer

> Please review an enhancement to detect [JavaScript 
> modules](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules)
>  when added to `javadoc` with the `--add-script` option, which require a 
> different `type` attribute in the HTML `

Re: RFR: 8329617: Update stylesheet for specs and tool documentation

2024-05-15 Thread Hannes Wallnöfer

On Tue, 14 May 2024 22:25:13 GMT, Jonathan Gibbons  wrote:

> Is there a reason to want the main font size to be different between the API 
> pages and spec pages?

I did not want to change the font size in this patch. I started by changing the 
font size to 14 px, but went back to the initial 10 pt in order to detect any 
unintentional changes. To me the specs/man pages look good with the original 
font size, so I kept it that way. We can increase it if we think it's too small.

> FWIW, while the PR description says the body font size is 10pt vs 14px, on 
> representative text for the `jdk.javadoc` module and Doc Comment Spec, I 
> observed 13.333px vs 14px.

That's expected, as [10 pt corresponds to 13.33 
px](https://cssunitconverter.com/pt-to-px/).

-

PR Comment: https://git.openjdk.org/jdk/pull/18694#issuecomment-2112330586

Integrated: 8294880: Review running time of jdk/internal/shellsupport/doc/JavadocHelperTest.java

2024-05-15 Thread Hannes Wallnöfer

On Thu, 18 Apr 2024 12:11:56 GMT, Hannes Wallnöfer  wrote:

> Please review a change to move a long-running shellsupport test from 
> langtools tier1 to tier2, while modifying the tier1 test to only retrieve doc 
> comments for members of `java.lang.StringBuilder`, which is where [the 
> original bug](https://bugs.openjdk.org/browse/JDK-8189778) occurred.  This 
> reduces the running time in the tier1 test from around 90 seconds to 2 
> seconds on my computer. The tier1 test also adds a non-null check for 
> retrieved doc comments.
> 
> I used the existing `langtools_jshell_unstable` test group to add the new 
> test in langtools tier2, which is not a perfect match for the test, but close 
> enough and helps to keep langtools `TEST.groups` file simple.

This pull request has now been integrated.

Changeset: fa043aec
Author:Hannes Wallnöfer 
URL:   
https://git.openjdk.org/jdk/commit/fa043aec425ae1e3086d09492b3fabcfbd3fa779
Stats: 96 lines in 3 files changed: 87 ins; 0 del; 9 mod

8294880: Review running time of 
jdk/internal/shellsupport/doc/JavadocHelperTest.java

Reviewed-by: jjg

-

PR: https://git.openjdk.org/jdk/pull/18837

Integrated: 8332080: Update troff man page for javadoc

2024-05-13 Thread Hannes Wallnöfer

On Fri, 10 May 2024 16:15:36 GMT, Hannes Wallnöfer  wrote:

> Please review a patch to update to the javadoc man page to add the 
> `--no-fonts` option and fix a typo.

This pull request has now been integrated.

Changeset: 1484153c
Author:Hannes Wallnöfer 
URL:   
https://git.openjdk.org/jdk/commit/1484153c1a092cefc20270b35aa1e508280843a4
Stats: 8 lines in 2 files changed: 5 ins; 0 del; 3 mod

8332080: Update troff man page for javadoc

Reviewed-by: jjg

-

PR: https://git.openjdk.org/jdk/pull/19181

RFR: 8332080: Update troff man page for javadoc

2024-05-10 Thread Hannes Wallnöfer

Please review a patch to update to the javadoc man page to add the `--no-fonts` 
option and fix a typo.

-

Commit messages:
 - JDK-8332080: Update troff man page for javadoc

Changes: https://git.openjdk.org/jdk/pull/19181/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19181&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8332080
  Stats: 8 lines in 2 files changed: 5 ins; 0 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/19181.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19181/head:pull/19181

PR: https://git.openjdk.org/jdk/pull/19181

Re: RFR: 8294880: Review running time of jdk/internal/shellsupport/doc/JavadocHelperTest.java [v3]

2024-05-10 Thread Hannes Wallnöfer

On Fri, 10 May 2024 13:42:43 GMT, Hannes Wallnöfer  wrote:

>> Please review a change to move a long-running shellsupport test from 
>> langtools tier1 to tier2, while modifying the tier1 test to only retrieve 
>> doc comments for members of `java.lang.StringBuilder`, which is where [the 
>> original bug](https://bugs.openjdk.org/browse/JDK-8189778) occurred.  This 
>> reduces the running time in the tier1 test from around 90 seconds to 2 
>> seconds on my computer. The tier1 test also adds a non-null check for 
>> retrieved doc comments.
>> 
>> I used the existing `langtools_jshell_unstable` test group to add the new 
>> test in langtools tier2, which is not a perfect match for the test, but 
>> close enough and helps to keep langtools `TEST.groups` file simple.
>
> Hannes Wallnöfer has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Change quick test to run on a random subset of classes

I have updated the PR to use a random subset of classes in the quick tier 1 
test. It now retrieves doc comments on 2% of classes, which corresponds to 
roughly 140 classes and takes around 7 seconds on my computer.

-

PR Comment: https://git.openjdk.org/jdk/pull/18837#issuecomment-2104639693

Re: RFR: 8294880: Review running time of jdk/internal/shellsupport/doc/JavadocHelperTest.java [v3]

2024-05-10 Thread Hannes Wallnöfer

> Please review a change to move a long-running shellsupport test from 
> langtools tier1 to tier2, while modifying the tier1 test to only retrieve doc 
> comments for members of `java.lang.StringBuilder`, which is where [the 
> original bug](https://bugs.openjdk.org/browse/JDK-8189778) occurred.  This 
> reduces the running time in the tier1 test from around 90 seconds to 2 
> seconds on my computer. The tier1 test also adds a non-null check for 
> retrieved doc comments.
> 
> I used the existing `langtools_jshell_unstable` test group to add the new 
> test in langtools tier2, which is not a perfect match for the test, but close 
> enough and helps to keep langtools `TEST.groups` file simple.

Hannes Wallnöfer has updated the pull request incrementally with one additional 
commit since the last revision:

  Change quick test to run on a random subset of classes

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18837/files
  - new: https://git.openjdk.org/jdk/pull/18837/files/7a45c94d..d400cf1b

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=18837&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18837&range=01-02

  Stats: 132 lines in 2 files changed: 48 ins; 72 del; 12 mod
  Patch: https://git.openjdk.org/jdk/pull/18837.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18837/head:pull/18837

PR: https://git.openjdk.org/jdk/pull/18837

Re: RFR: 8317621: --add-script should support JavaScript modules

2024-05-07 Thread Hannes Wallnöfer

On Fri, 29 Mar 2024 09:39:18 GMT, Hannes Wallnöfer  wrote:

> Please review an enhancement to detect [JavaScript 
> modules](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules)
>  when added to `javadoc` with the `--add-script` option, which require a 
> different `type` attribute in the HTML `

RFR: 8317621: --add-script should support JavaScript modules

2024-05-07 Thread Hannes Wallnöfer

Please review an enhancement to detect [JavaScript 
modules](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules) 
when added to `javadoc` with the `--add-script` option, which require a 
different `type` attribute in the HTML `

Integrated: 8296175: Output warning if generated docs contain diagnostic markers

2024-04-26 Thread Hannes Wallnöfer

On Fri, 26 Apr 2024 15:14:04 GMT, Hannes Wallnöfer  wrote:

> Please review a change to print a terminal message if the documentation 
> generated by `HtmlDoclet` contains any diagnostic marker elements for invalid 
> input. The message is printed after the documentation has been generated and 
> before the error/warning count with the purpose to let the user know that the 
> documentation is not ready to be deployed and can be consulted for locating 
> invalid input. 
> 
> Example `javadoc` output containing the new message:
> 
> 
> ...
> Generating out/index.html...
> Generating out/help-doc.html...
> The generated documentation contains diagnostic markers for invalid input.
> 3 errors
> 1 warning

This pull request has now been integrated.

Changeset: aee91fbc
Author:Hannes Wallnöfer 
URL:   
https://git.openjdk.org/jdk/commit/aee91fbc70df716b96c202511b4ff1b302df8d60
Stats: 44 lines in 5 files changed: 32 ins; 0 del; 12 mod

8296175: Output warning if generated docs contain diagnostic markers

Reviewed-by: jjg

-

PR: https://git.openjdk.org/jdk/pull/18980

Re: RFR: 8296175: Output warning if generated docs contain diagnostic markers

2024-04-26 Thread Hannes Wallnöfer

On Fri, 26 Apr 2024 18:05:14 GMT, Jonathan Gibbons  wrote:

> The feature is good, but I'm surprised to see it being tested in 
> DocLintTest.java.

I admit I took a shortcut there, using `DocLintTest` for something it wasn't 
meant to do. The test had to be updated anyway, and it had both the invalid 
input and the output checks I needed, so I took advantage.

-

PR Comment: https://git.openjdk.org/jdk/pull/18980#issuecomment-2079958979

RFR: 8296175: Output warning if generated docs contain diagnostic markers

2024-04-26 Thread Hannes Wallnöfer

Please review a change to print a terminal message if the documentation 
generated by `HtmlDoclet` contains any diagnostic marker elements for invalid 
input. The message is printed after the documentation has been generated and 
before the error/warning count with the purpose to let the user know that the 
documentation is not ready to be deployed and can be consulted for locating 
invalid input. 

Example `javadoc` output containing the new message:


...
Generating out/index.html...
Generating out/help-doc.html...
The generated documentation contains diagnostic markers for invalid input.
3 errors
1 warning

-

Commit messages:
 - JDK-8296175: Output warning if generated docs contain diagnostic markers

Changes: https://git.openjdk.org/jdk/pull/18980/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18980&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8296175
  Stats: 44 lines in 5 files changed: 32 ins; 0 del; 12 mod
  Patch: https://git.openjdk.org/jdk/pull/18980.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18980/head:pull/18980

PR: https://git.openjdk.org/jdk/pull/18980

Re: RFR: 8294880: Review running time of jdk/internal/shellsupport/doc/JavadocHelperTest.java [v2]

2024-04-23 Thread Hannes Wallnöfer

> Please review a change to move a long-running shellsupport test from 
> langtools tier1 to tier2, while modifying the tier1 test to only retrieve doc 
> comments for members of `java.lang.StringBuilder`, which is where [the 
> original bug](https://bugs.openjdk.org/browse/JDK-8189778) occurred.  This 
> reduces the running time in the tier1 test from around 90 seconds to 2 
> seconds on my computer. The tier1 test also adds a non-null check for 
> retrieved doc comments.
> 
> I used the existing `langtools_jshell_unstable` test group to add the new 
> test in langtools tier2, which is not a perfect match for the test, but close 
> enough and helps to keep langtools `TEST.groups` file simple.

Hannes Wallnöfer has updated the pull request incrementally with one additional 
commit since the last revision:

  Move FullJavaDocHelperTest.java into its own group

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18837/files
  - new: https://git.openjdk.org/jdk/pull/18837/files/340a3b4f..7a45c94d

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=18837&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18837&range=00-01

  Stats: 9 lines in 1 file changed: 5 ins; 1 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/18837.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18837/head:pull/18837

PR: https://git.openjdk.org/jdk/pull/18837

Integrated: 8330063: Upgrade jQuery to 3.7.1

2024-04-19 Thread Hannes Wallnöfer

On Fri, 12 Apr 2024 11:11:47 GMT, Hannes Wallnöfer  wrote:

> Please review a change to update jQuery from version 3.6.1 to 3.7.1 in 
> JavaDoc. The jQuery compressed and uncompressed files were obtained from 
> https://jquery.com/download/. Changes were tested against javadoc tests as 
> well as manually using a selection of browsers (Firefox, Chrome, Safari). 
> 
> [As of jQuery 
> 3.7.0](https://blog.jquery.com/2023/05/11/jquery-3-7-0-released-staying-in-order/#:~:text=jQuery%20no%20longer%20depends%20on%20Sizzle%20as%20a%20separate%20project%2C%20but%20has%20instead%20dropped%20its%20code%20directly%20into%20jQuery%20core.)
>  Sizzle is longer bundled as an independent project. The code is now part of 
> jQuery core, therefore the Sizzle license has been removed from `jquery.md`.

This pull request has now been integrated.

Changeset: 46a2ce4e
Author:Hannes Wallnöfer 
URL:   
https://git.openjdk.org/jdk/commit/46a2ce4eb4c961649a70d52a6317a2c022e87d63
Stats: 21689 lines in 9 files changed: 10724 ins; 10957 del; 8 mod

8330063: Upgrade jQuery to 3.7.1

Reviewed-by: prappo

-

PR: https://git.openjdk.org/jdk/pull/18756

RFR: 8294880: Review running time of jdk/internal/shellsupport/doc/JavadocHelperTest.java

2024-04-18 Thread Hannes Wallnöfer

Please review a change to move a long-running shellsupport test from langtools 
tier1 to tier2, while modifying the tier1 test to only retrieve doc comments 
for members of `java.lang.StringBuilder`, which is where [the original 
bug](https://bugs.openjdk.org/browse/JDK-8189778) occurred.  This reduces the 
running time in the tier1 test from around 90 seconds to 2 seconds on my 
computer. The tier1 test also adds a non-null check for retrieved doc comments.

I used the existing `langtools_jshell_unstable` test group to add the new test 
in langtools tier2, which is not a perfect match for the test, but close enough 
and helps to keep langtools `TEST.groups` file simple.

-

Commit messages:
 - JDK-8294880: Review running time of 
jdk/internal/shellsupport/doc/JavadocHelperTest.java

Changes: https://git.openjdk.org/jdk/pull/18837/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18837&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8294880
  Stats: 157 lines in 3 files changed: 126 ins; 19 del; 12 mod
  Patch: https://git.openjdk.org/jdk/pull/18837.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18837/head:pull/18837

PR: https://git.openjdk.org/jdk/pull/18837

Re: RFR: 8330063: Upgrade jQuery to 3.7.1

2024-04-17 Thread Hannes Wallnöfer

On Fri, 12 Apr 2024 11:11:47 GMT, Hannes Wallnöfer  wrote:

> Please review a change to update jQuery from version 3.6.1 to 3.7.1 in 
> JavaDoc. The jQuery compressed and uncompressed files were obtained from 
> https://jquery.com/download/. Changes were tested against javadoc tests as 
> well as manually using a selection of browsers (Firefox, Chrome, Safari). 
> 
> [As of jQuery 
> 3.7.0](https://blog.jquery.com/2023/05/11/jquery-3-7-0-released-staying-in-order/#:~:text=jQuery%20no%20longer%20depends%20on%20Sizzle%20as%20a%20separate%20project%2C%20but%20has%20instead%20dropped%20its%20code%20directly%20into%20jQuery%20core.)
>  Sizzle is longer bundled as an independent project. The code is now part of 
> jQuery core, therefore the Sizzle license has been removed from `jquery.md`.

In the jQuery.js file the Sizzle.js copyright notice used to be somewhere 
within the file near the Sizzle code.

-

PR Comment: https://git.openjdk.org/jdk/pull/18756#issuecomment-2061267529

Re: RFR: 8330063: Upgrade jQuery to 3.7.1

2024-04-17 Thread Hannes Wallnöfer

On Fri, 12 Apr 2024 11:11:47 GMT, Hannes Wallnöfer  wrote:

> Please review a change to update jQuery from version 3.6.1 to 3.7.1 in 
> JavaDoc. The jQuery compressed and uncompressed files were obtained from 
> https://jquery.com/download/. Changes were tested against javadoc tests as 
> well as manually using a selection of browsers (Firefox, Chrome, Safari). 
> 
> [As of jQuery 
> 3.7.0](https://blog.jquery.com/2023/05/11/jquery-3-7-0-released-staying-in-order/#:~:text=jQuery%20no%20longer%20depends%20on%20Sizzle%20as%20a%20separate%20project%2C%20but%20has%20instead%20dropped%20its%20code%20directly%20into%20jQuery%20core.)
>  Sizzle is longer bundled as an independent project. The code is now part of 
> jQuery core, therefore the Sizzle license has been removed from `jquery.md`.

The Sizzle.js license used to be contained in `external/sizzle/LICENSE.txt`. 
The file was [removed in the process of inlining 
Sizzle.js](https://github.com/jquery/jquery/pull/5113/files#diff-df1327f58759ffed4e68f82e113a90e4fb38b8d17ab4e4bf0de7c1824a6c454f).

-

PR Comment: https://git.openjdk.org/jdk/pull/18756#issuecomment-2061252990

RFR: 8330063: Upgrade jQuery to 3.7.1

2024-04-12 Thread Hannes Wallnöfer

Please review a change to update jQuery from version 3.6.1 to 3.7.1 in JavaDoc. 
The jQuery compressed and uncompressed files were obtained from 
https://jquery.com/download/. Changes were tested against javadoc tests as well 
as manually using a selection of browsers (Firefox, Chrome, Safari). 

[As of jQuery 
3.7.0](https://blog.jquery.com/2023/05/11/jquery-3-7-0-released-staying-in-order/#:~:text=jQuery%20no%20longer%20depends%20on%20Sizzle%20as%20a%20separate%20project%2C%20but%20has%20instead%20dropped%20its%20code%20directly%20into%20jQuery%20core.)
 Sizzle is longer bundled as an independent project. The code is now part of 
jQuery core, therefore the Sizzle license has been removed from `jquery.md`.

-

Commit messages:
 - JDK-8330063: Upgrade jQuery to 3.7.1

Changes: https://git.openjdk.org/jdk/pull/18756/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18756&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8330063
  Stats: 21689 lines in 9 files changed: 10724 ins; 10957 del; 8 mod
  Patch: https://git.openjdk.org/jdk/pull/18756.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18756/head:pull/18756

PR: https://git.openjdk.org/jdk/pull/18756

RFR: JDK-8329617: Update stylesheet for specs and tool documentation

2024-04-09 Thread Hannes Wallnöfer

Please review an update to the `jdk-default.css` stylesheet used for 
specifications and tool guides. The original purpose was to make use of the 
Dejavu web fonts provided by the API docs and to update the navigation bar to 
match the one in the API docs. However, the updates include some other fixes 
and improvements also described below.

 - The change to use the DejaVu web fonts consists only of the `@import` 
statement in line 16 as the stylesheet already used DejaVu web fonts as first 
choice in its `font-family` rules.
 - The changes to make the navigation bar match the one in the API docs are 
mostly located at the end of the file (beyond line 160). However, this also 
includes setting the `margin` property to '0' in the `body` element and adding 
a `margin` in the `main` and `footer` elements instead. 
 - To set the horizontal margin for page content elements outside the `main` 
element which occur in some pages, a margin is set explicitly on those elements 
in lines 48-50. While this is a bit awkward, I think it's still better than 
working with negative margins in the header to offset the margin in the `body` 
element.
 - Most of the remaining changes (lines 53-110) are changes are to redefine the 
styles in simpler terms, such as leaving out declarations that are equal to 
browser defaults, and removing the units from `0`-length values.

The changes are intended to preserve the layout of the pages, including the 
body font size which is slightly different from the one used in API docs 
(`10pt` vs `14px`). I can provide before/after snapshots of the rendered 
documentation if desired.

-

Commit messages:
 - JDK-8329617: Update stylesheet for specs and tool documentation

Changes: https://git.openjdk.org/jdk/pull/18694/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18694&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8329617
  Stats: 73 lines in 1 file changed: 27 ins; 19 del; 27 mod
  Patch: https://git.openjdk.org/jdk/pull/18694.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18694/head:pull/18694

PR: https://git.openjdk.org/jdk/pull/18694

Re: RFR: JDK-8324342: doclet should default @since for a nested class to that of its enclosing class

2024-04-08 Thread Hannes Wallnöfer

On Wed, 20 Mar 2024 19:04:04 GMT, Jonathan Gibbons  wrote:

> Please review changes to the support for `since`, `author`, and `version` 
> tags, such that if there are no such tags on a nested class, the doclet will 
> look for any such tags in enclosing classes.

Looks good. I'll leave it to you and @pavelrappo to decide whether to include 
the partially unrelated updates to the man page.

-

Marked as reviewed by hannesw (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18407#pullrequestreview-1986947427

Integrated: JDK-8324774: Add DejaVu web fonts

2024-03-27 Thread Hannes Wallnöfer

On Tue, 30 Jan 2024 16:13:42 GMT, Hannes Wallnöfer  wrote:

> This change adds the DejaVu web fonts that were previously maintained 
> externally to the open repository so they are available both in JDK API 
> documentation and any API documentation generated with the `javadoc` tool. 
> All files added in this PR are the same as the ones previously maintained 
> externally, with the exception of added license and name/version comments in 
> `dejavu.css`.
> 
> Copying of font files to the generated documentation is done by looking for 
> font file names in `dejavu.css`, so font file names can be changed without 
> changing the code. However, the font file list is hard-coded in 
> `APITest.java`. `CheckLibraryVersions.java` is updated to make sure the name 
> and version in the legal file matches the one in the stylesheet. Of course I 
> also performed manual tests to make sure the font and legal files are copied 
> to the output tree and used correctly in browsers.
> 
> Once #17411 is integrated, `dejavu.css` should also be added to the list of 
> files checked by the new "pass-through" test.

This pull request has now been integrated.

Changeset: d0a26503
Author:Hannes Wallnöfer 
URL:   
https://git.openjdk.org/jdk/commit/d0a265039a36292d87b249af0e8977982e5acc7b
Stats: 546 lines in 38 files changed: 533 ins; 0 del; 13 mod

8324774: Add DejaVu web fonts
8327385: Add JavaDoc option to exclude web fonts from generated documentation

Reviewed-by: ihse, jjg

-

PR: https://git.openjdk.org/jdk/pull/17633

Re: RFR: JDK-8324774: Add DejaVu web fonts [v2]

2024-03-20 Thread Hannes Wallnöfer

> This change adds the DejaVu web fonts that were previously maintained 
> externally to the open repository so they are available both in JDK API 
> documentation and any API documentation generated with the `javadoc` tool. 
> All files added in this PR are the same as the ones previously maintained 
> externally, with the exception of added license and name/version comments in 
> `dejavu.css`.
> 
> Copying of font files to the generated documentation is done by looking for 
> font file names in `dejavu.css`, so font file names can be changed without 
> changing the code. However, the font file list is hard-coded in 
> `APITest.java`. `CheckLibraryVersions.java` is updated to make sure the name 
> and version in the legal file matches the one in the stylesheet. Of course I 
> also performed manual tests to make sure the font and legal files are copied 
> to the output tree and used correctly in browsers.
> 
> Once #17411 is integrated, `dejavu.css` should also be added to the list of 
> files checked by the new "pass-through" test.

Hannes Wallnöfer has updated the pull request incrementally with two additional 
commits since the last revision:

 - JDK-8327385: Add JavaDoc option to exclude web fonts from generated 
documentation
 - Merge try-with-resource statements

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17633/files
  - new: https://git.openjdk.org/jdk/pull/17633/files/2c6e2163..9f77de97

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=17633&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17633&range=00-01

  Stats: 188 lines in 7 files changed: 174 ins; 2 del; 12 mod
  Patch: https://git.openjdk.org/jdk/pull/17633.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17633/head:pull/17633

PR: https://git.openjdk.org/jdk/pull/17633

Integrated: JDK-8325874: Improve checkbox-based interface in summary pages

On Mon, 11 Mar 2024 13:37:54 GMT, Hannes Wallnöfer  wrote:

> Please review a change to improve the user experience and implementation for 
> the checkbox-based interface to selectively display content on API summary 
> pages:
> 
>  - Add a checkbox to toggle (select/unselect) all checkboxes in the page.
>  - Remove the `Table.setAlwaysShowDefaultTab(boolean)` setter as it is 
> redundant and can be replaced by `!renderTabs`.
>  - Consolidate the code to generate checkboxes into a new protected 
> `SummaryListWriter.getCheckbox` method.

This pull request has now been integrated.

Changeset: 03532450
Author:Hannes Wallnöfer 
URL:   
https://git.openjdk.org/jdk/commit/035324503f5e04b53d99573a664fd1367b7ccf30
Stats: 142 lines in 9 files changed: 55 ins; 52 del; 35 mod

8325874: Improve checkbox-based interface in summary pages

Reviewed-by: prappo

-

PR: https://git.openjdk.org/jdk/pull/18194

Re: RFR: JDK-8325874: Improve checkbox-based interface in summary pages [v2]

> Please review a change to improve the user experience and implementation for 
> the checkbox-based interface to selectively display content on API summary 
> pages:
> 
>  - Add a checkbox to toggle (select/unselect) all checkboxes in the page.
>  - Remove the `Table.setAlwaysShowDefaultTab(boolean)` setter as it is 
> redundant and can be replaced by `!renderTabs`.
>  - Consolidate the code to generate checkboxes into a new protected 
> `SummaryListWriter.getCheckbox` method.

Hannes Wallnöfer has updated the pull request incrementally with one additional 
commit since the last revision:

  Review feedback

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18194/files
  - new: https://git.openjdk.org/jdk/pull/18194/files/cb37764d..d7460e8d

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=18194&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18194&range=00-01

  Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/18194.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18194/head:pull/18194

PR: https://git.openjdk.org/jdk/pull/18194

Re: RFR: JDK-8325874: Improve checkbox-based interface in summary pages

On Mon, 11 Mar 2024 18:09:41 GMT, Pavel Rappo  wrote:

> if you check "all" and then uncheck something else, then "all" will remain 
> selected. That is a confusing state.

I agree, but I think the behavior of the checkbox is as it should be, and it's 
the label that is slightly misleading. The intended behavior of the checkbox is 
set all other checkboxes to a unified state which can be either "on" or "off", 
not to reflect the state of other checkboxes. 

So maybe the correct label would be "toggle all" rather than just "all". 
Admittedly I have used the shorter "all" label to limit the length of the 
already long line of checkboxes. Do you think using "toggle all" as label would 
make things better?

-

PR Comment: https://git.openjdk.org/jdk/pull/18194#issuecomment-1994536241

Re: RFR: JDK-8328006: refactor large anonymous inner class in HtmlDocletWriter