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

[Nizar Benalla]

On Fri, 14 Jun 2024 12:12:51 GMT, Nizar Benalla <nbenalla at openjdk.org> wrote:

>> Can I please get a review for this change, that aims to add support for Global HTML tags.
>> Here is the [link](https://cr.openjdk.org/~nbenalla/javadocGlobalPR/pkg1/package-summary.html) to the generated docs.
>> Thanks in advance.
>
> Nizar Benalla 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 ten additional commits since the last revision:
> 
>  - Null safety
>  - Merge branch 'master' into html-attributes
>  - remove trailing whitespace
>  - -Add a boolean attribute to the enum type
>    -Simply regex in `visitAttribute`
>    -Simplified the Test
>    -Added a negative test
>  - no longer print summary
>  - no longer print summary
>  - Add small comment
>  - Remove classpath exception
>  - Allow global variables

I've updated the PR based on everyone's suggestions, here are the main changes.
Currently running tier 1-3

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java line 695:

> 693:             // custom "data-*" attributes are also accepted
> 694:             var attrName = name.toString();
> 695:             if (!attrName.startsWith("on") && !attrName.startsWith("data-")) {

I've removed the regex here.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/HtmlTag.java line 441:

> 439:         ALINK,
> 440:         ALT,
> 441:         ARIA_ACTIVEDESCENDANT(true),

The enum `Attr` holds two values now, the second representing whether it's a global attribute.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/HtmlTag.java line 670:

> 668:     public AttrKind getAttrKind(Name attrName) {
> 669:         Attr attr= getAttr(attrName);
> 670:         AttrKind k = attr.isGlobal()? AttrKind.OK: attrs.get(attr); // null-safe

As hannes suggested, `getAttrKind` now returns `AttrKind.OK` if we are dealing with a global attribute.

Edit: It's no longer null safe so I will slightly change it

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/HtmlTag.java line 673:

> 671: 
> 672:         if (attr != null) {
> 673:             kind = attr.isGlobal() ? AttrKind.OK : attrs.get(attr);  // get is null-safe

As hannes suggested, `getAttrKind` now returns `AttrKind.OK` if we are dealing with a global attribute.

I've added a null check here because of `attr.isGlobal()`

test/langtools/jdk/javadoc/doclet/TestGlobalHtml/TestGlobalHtml.java line 70:

> 68:                 public class C {
> 69:                     /**
> 70:                      * <form>

Added a negative test, as `<a>` and `<form>` tags aren't allowed .

test/langtools/jdk/javadoc/doclet/TestGlobalHtml/pkg1/C1.java line 23:

> 21:  * questions.
> 22:  */
> 23: 

I've simplified the `C1` class, it's smaller now and has many smaller examples.

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

PR Review: https://git.openjdk.org/jdk/pull/19652#pullrequestreview-2117318623
PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1639144111
PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1639145147
PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1639147374
PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1639726374
PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1639145878
PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1639146304