On Tue, 11 Jun 2024 13:26:28 GMT, Pavel Rappo <pra...@openjdk.org> wrote:
>> 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 > > 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.matches("data-[a-z]+(-[a-z]+)*")){ > > I understand that `startsWith("on")` precedes this PR. However, it's more > permissive than it should be: in reality, there are only 70 global attributes > that start with "on". On the other hand, > `attrName.matches("data-[a-z]+(-[a-z]+)*")` is less permissive. For example, > if I read the XML spec correctly, such an attribute can contain `_` or start > with `data-.`. > > But maybe those checks are good enough? I think like being slightly restrictive and safe. > test/langtools/jdk/javadoc/doclet/TestGlobalHtml/TestGlobalHtml.java line 41: > >> 39: var tester = new TestGlobalHtml(); >> 40: tester.runTests(); >> 41: tester.printSummary(); > > Why is this summary needed? Usually, we don't print it. Fixed in [cf4dab6](https://github.com/openjdk/jdk/pull/19652/commits/cf4dab6ceefb8625e9e7b281073cba49b894d13c). I was using it locally. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1635017243 PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1634982192