On Fri, 31 Jul 2026 19:56:02 GMT, Shiv Shah <[email protected]> wrote:
> The failure handler keys each HTML section id by the command's name path, so
> when a command name occurs again after an intervening command - the same
> command repeated, or the same top-level tool reused, as in
> "jcmd.compiler.codecache jinfo jcmd.compiler.codelist" - a second section
> with the same id is written. Anchors and the data-toggle / data-show
> javascript resolve ids via document.getElementById, which returns the first
> match, so the later occurrence's output is unreachable. These are the two
> cases reported in the bug.
>
> Fix: the root section keeps a registry of issued ids; a repeated id gets the
> first free numeric suffix ("jcmd-2", ...). Displayed names are unchanged,
> adjacent commands with a shared prefix still share their sections, children
> of a suffixed section are unique automatically, and the registry also
> prevents collisions with a command literally named like a suffix. The only
> external id reconstruction, ToolKit.link(), is keyed by PID and unaffected.
>
> Ids can still repeat across appended fragments (each invocation appends a
> complete HTML document); that is pre-existing and tracked as JDK-8389541.
>
> Verified with a standalone driver using both sequences from the bug: six
> duplicated ids before the fix, none after, every run's output reachable. New
> HtmlSectionTest (plain JUnit, like the existing unit tests) covers both
> reported cases, per-occurrence output association, triple repetition, the
> "jcmd-2" name collision, and the unchanged adjacent / shared-prefix behavior
> - 6/6 pass.
>
>
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK
> Interim AI Policy](https://openjdk.org/legal/ai).
test/failure_handler/src/share/classes/jdk/test/failurehandler/HtmlSection.java
line 75:
> 73: } else {
> 74: this.rootSection = rootSection;
> 75: this.sectionIds = null;
Is setting to `null` necessary or can we always initialize `sectionIds` to an
empty `HashSet`? It makes things a bit easier at call sites and avoids null
checks. On a related note, the place where this `sectionIds` is currently being
used doesn't do a null check, so that might be another reason to just always
initialize this set to a non-null value.
test/failure_handler/test/unit/jdk/test/failurehandler/HtmlSectionTest.java
line 27:
> 25:
> 26: import org.junit.Assert;
> 27: import org.junit.Test;
This looks like an outdated version of JUnit. Several parts of the JDK use
JUnit 5 (through jtreg). I don't see any jtreg test tag definitions on this
test either. How are these failure handler tests launched?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32157#discussion_r3735548082
PR Review Comment: https://git.openjdk.org/jdk/pull/32157#discussion_r3735558345
