Skip to content

Fix sidebar navigation for non-ASCII localized section headers#26068

Merged
MarkEWaite merged 14 commits intojenkinsci:masterfrom
AmoghParmar:fix-23885-sidebar-navigation
Jan 21, 2026
Merged

Fix sidebar navigation for non-ASCII localized section headers#26068
MarkEWaite merged 14 commits intojenkinsci:masterfrom
AmoghParmar:fix-23885-sidebar-navigation

Conversation

@AmoghParmar
Copy link
Contributor

@AmoghParmar AmoghParmar commented Jan 5, 2026

Fixes #23885

Testing done

This change was manually verified by running Jenkins locally from source with the Japanese locale enabled.

Steps:

  1. Built Jenkins from the fix-23885-sidebar-navigation branch.
  2. Opened a job’s Configure page with the UI language set to Japanese.
  3. Clicked ビルド後の処理 (Post-build Actions) in the left sidebar.
  4. Verified that the page scrolls to the correct section instead of ソースコード管理 (Source Code Management).

This confirms that localized section headers now generate unique DOM IDs and sidebar navigation behaves correctly.

Screenshots (UI changes only)

Before

Clicking ビルド後の処理 scrolls to ソースコード管理 due to both sections resolving to the same DOM ID.

image

After

Clicking ビルド後の処理 scrolls to the correct section.

image

Proposed changelog entries

  • Fix sidebar navigation for non-ASCII localized section headers.

Proposed changelog category

/label bug, web-ui

Proposed upgrade guidelines

N/A

Submitter checklist

  • The issue, if it exists, is well-described.
  • The changelog entries and upgrade guidelines are appropriate for the audience affected by the change and are in the imperative mood.
  • There is automated testing or an explanation as to why this change has no tests.
  • New public classes, fields, and methods are annotated with @Restricted or have @since Javadocs, as appropriate.
  • New deprecations are annotated appropriately, if applicable.
  • UI changes do not introduce regressions with Content Security Policy.
  • For dependency updates, links to external changelogs are provided, if applicable.
  • For new APIs and extension points, there is a link to at least one consumer, if applicable.

Desired reviewers

@jenkinsci/core-pr-reviewers

@comment-ops-bot comment-ops-bot bot added bug For changelog: Minor bug. Will be listed after features web-ui The PR includes WebUI changes which may need special expertise labels Jan 5, 2026
Comment on lines 11 to 14
// If ASCII stripping produces something meaningful, keep old behavior
if (ascii && ascii !== "-") {
return ascii;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this still here? This seems unnecessary and prone to problems. In particular, a similar problem would occur if there are sections with partially ASCII words.

Admittedly unlikely for the terminology we're using in the default sections, but plugins may add their own IIRC.

strangelookingnerd and others added 11 commits January 7, 2026 11:13
…6036)

* Update README.md

* Update README.md

* Update README.md

* Remove link to non-existent newsletter

Also format with `yarn lint:fix` as noted in the contributing guide.

* Update README.md

* Update good first issues query

* Fix race condition during initial admin account creation

* Update core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java

Co-authored-by: Copilot <[email protected]>

* Use synchronized method for initial admin account creation

---------

Co-authored-by: Mark Waite <[email protected]>
Co-authored-by: Kris Stern <[email protected]>
Co-authored-by: Copilot <[email protected]>
Reinstantiate telemetry for Java properties

Signed-off-by: Alexander Brandes <[email protected]>
…ci#26041)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…2.vb_1833c1231d3 (jenkinsci#26042)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…jenkinsci#26043)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…sci#26046)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Update stapler.version to v2065

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…6054)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@github-actions github-actions bot added the unresolved-merge-conflict There is a merge conflict with the target branch. label Jan 7, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2026

Please take a moment and address the merge conflicts of your pull request. Thanks!

@github-actions github-actions bot removed the unresolved-merge-conflict There is a merge conflict with the target branch. label Jan 7, 2026
@AmoghParmar
Copy link
Contributor Author

Hi @daniel-beck

This PR is hitting a failure in Security3630Test.testHashMap on Linux JDK 21. The failure seems to come from an unexpected update-center lookup (No download site found) rather than CLI logic touched by this change.

Does this look like a known CI / initialization-order issue, or is there a recommended way to avoid triggering this path from UI-related changes? Happy to follow up with fixes once I understand what the test expects.

Thanks!

@MarkEWaite
Copy link
Contributor

MarkEWaite commented Jan 12, 2026

This PR is hitting a failure in Security3630Test.testHashMap on Linux JDK 21. The failure seems to come from an unexpected update-center lookup (No download site found) rather than CLI logic touched by this change.

That is an unexpected failure, likely caused by the Jenkins update center being unavailable during the automated test.

Does this look like a known CI / initialization-order issue, or is there a recommended way to avoid triggering this path from UI-related changes? Happy to follow up with fixes once I understand what the test expects.

We expect the Jenkins update center to be up all the time. More than 250,000 distinct Jenkins controllers access it every week.

I've started a new build that should confirm the test failure is transient. Daniel updated the test to only run it on certain configurations as part of pull request:

We use Launchable to track test flakiness. That test has failed roughly 1 of 50 test runs as shown in this screenshot from Launchable:

hudson-cli-Security3630Test-2026-01-12-05_20_17-edit

@daniel-beck daniel-beck added the squash-merge-me Unclean or useless commit history, should be merged only with squash-merge label Jan 12, 2026
@MarkEWaite MarkEWaite added ath-successful This PR has successfully passed the full acceptance-test-harness suite pct-successful This PR has successfully passed the full plugin-compatibility-test suite labels Jan 19, 2026
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has passed ATH and PCT testing and has been interactively tested by the submitter. Thanks!

Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback.

/label ready-for-merge

@comment-ops-bot comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Jan 20, 2026
@MarkEWaite
Copy link
Contributor

This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback.

/label ready-for-merge

I am withdrawing my ready-for-merge because the new code always encodes characters, even ASCII characters. For instance, if I use " abc def gh " as the string, the old code generated the identifier "abc-def-gh" while the new code generates the identifier "61-62-63-20-64-65-66-20-67-68".

Is that intended @AmoghParmar ? Have you confirmed the safety of that change by testing and by code review?

@MarkEWaite MarkEWaite removed the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Jan 20, 2026
@MarkEWaite MarkEWaite self-requested a review January 20, 2026 02:13
@daniel-beck
Copy link
Member

daniel-beck commented Jan 20, 2026

@MarkEWaite In the current UI, the page does not link to these as anchors (i.e., they do not show up in the URL), so there seems to be no negative side effects of encoding them like this. Could you clarify what your concerns are?

If we ever want to link there from elsewhere, we'd probably need to switch to locale-independent IDs anyway.

@MarkEWaite
Copy link
Contributor

the page does not link to these as anchors (i.e., they do not show up in the URL), so there seems to be no negative side effects of encoding them like this. Could you clarify what your concerns are?

My concern was that there might be links that refer to these ID values as anchors. I had not considered that if there were links that refer to these ID values, those links would be broken by any localization of the text. Thanks for the clarification.

@daniel-beck
Copy link
Member

@MarkEWaite
Does that mean we can agree on 9d78384 / https://github.com/jenkinsci/jenkins/pull/26068/files/9d78384173b4149323c85e935619d4c48ed447c0 as the reasonable state for this PR we can accept?
For outside references, the anchors would need to be in a default locale anyway.

IMO: The latest state only works for locales like English; even Spanish will have the majority of IDs as unreadable strings, so it's not much better than ignoring the "problem" entirely while having more complexity. A different encoding (with behavior perhaps similar to Punycode) would be needed to make partially ASCII strings readable, but I don't think it's worth the effort or complexity.

@MarkEWaite
Copy link
Contributor

@MarkEWaite
Does that mean we can agree on 9d78384 / https://github.com/jenkinsci/jenkins/pull/26068/files/9d78384173b4149323c85e935619d4c48ed447c0 as the reasonable state for this PR we can accept?
For outside references, the anchors would need to be in a default locale anyway

Yes. I think that the most recent change should be reverted.

@AmoghParmar AmoghParmar force-pushed the fix-23885-sidebar-navigation branch from 56fe95f to 9d78384 Compare January 20, 2026 18:43
@AmoghParmar
Copy link
Contributor Author

Hi @MarkEWaite and @daniel-beck,

I’ve reverted to the full encoding approach, as discussed.
Thank you both for the detailed discussion and guidance on the best approach!

Copy link
Member

@daniel-beck daniel-beck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well in my local testing, both with Spanish and Traditional Chinese locales. Unlike current releases, I can jump to the last section of a freestyle config in Traditional Chinese now (weekly gets stuck at the first translated entry).

@daniel-beck
Copy link
Member

@AmoghParmar Thanks for your patience and willingness to adapt, while we figure out how this should be fixed!

@AmoghParmar
Copy link
Contributor Author

@AmoghParmar Thanks for your patience and willingness to adapt, while we figure out how this should be fixed!

Thanks @daniel-beck for testing and for the kind words.

Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback.

/label ready-for-merge

@comment-ops-bot comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Jan 20, 2026
@MarkEWaite MarkEWaite merged commit 33b3b3b into jenkinsci:master Jan 21, 2026
26 checks passed
@welcome
Copy link

welcome bot commented Jan 21, 2026

Congratulations on getting your very first Jenkins core pull request merged 🎉🥳

This is a fantastic achievement, and we're thrilled to have you as part of our community! Thank you for your valuable input, and we look forward to seeing more of your contributions in the future!

We would like to invite you to join the community chats and forums to meet other Jenkins contributors 😊
Don't forget to check out the participation page to learn more about how to contribute to Jenkins.


meetgoti07 pushed a commit to meetgoti07/jenkins that referenced this pull request Jan 29, 2026
…nsci#26068)

* Fix sidebar navigation for non-ASCII localized section headers

* Bump minimum supported remoting version (jenkinsci#26034)

* Fix race condition during initial admin account creation (jenkinsci#26036)

* Update README.md

* Update README.md

* Update README.md

* Remove link to non-existent newsletter

Also format with `yarn lint:fix` as noted in the contributing guide.

* Update README.md

* Update good first issues query

* Fix race condition during initial admin account creation

* Update core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java

Co-authored-by: Copilot <[email protected]>

* Use synchronized method for initial admin account creation

---------

Co-authored-by: Mark Waite <[email protected]>
Co-authored-by: Kris Stern <[email protected]>
Co-authored-by: Copilot <[email protected]>

* Collect telemetry for Java properties (jenkinsci#26038)

Reinstantiate telemetry for Java properties

Signed-off-by: Alexander Brandes <[email protected]>

* Update dependency com.puppycrawl.tools:checkstyle to v12.3.1 (jenkinsci#26041)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency io.jenkins.plugins:jakarta-xml-bind-api to v4.0.6-12.vb_1833c1231d3 (jenkinsci#26042)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency io.jenkins.plugins:jaxb to v2.3.9-143.v5979df3304e6 (jenkinsci#26043)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency io.jenkins.plugins:checks-api to v402 (jenkinsci#26044)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency org.jenkins-ci.plugins:credentials to v1460 (jenkinsci#26046)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update stapler.version to v2065 (major) (jenkinsci#26049)

Update stapler.version to v2065

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency org.jenkins-ci:version-number to v1.14 (jenkinsci#26054)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Fix sidebar navigation for non-ASCII section headers

* Resolve merge conflict with master

---------

Signed-off-by: Alexander Brandes <[email protected]>
Co-authored-by: strangelookingnerd <[email protected]>
Co-authored-by: Pnkcaht <[email protected]>
Co-authored-by: Mark Waite <[email protected]>
Co-authored-by: Kris Stern <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Alexander Brandes <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Daniel Beck <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ath-successful This PR has successfully passed the full acceptance-test-harness suite bug For changelog: Minor bug. Will be listed after features pct-successful This PR has successfully passed the full plugin-compatibility-test suite ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback squash-merge-me Unclean or useless commit history, should be merged only with squash-merge web-ui The PR includes WebUI changes which may need special expertise

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sidebar link is wrong for localized section headings

6 participants