Fix unsaved changes warning in new Manage Jenkins UI

[Heller07]

Fixes #26270
In the new Manage Jenkins UI, configuration pages rendered via settings-subpage.jelly did not include lib.form.confirm, so dirty-form tracking was not initialized and the native "unsaved changes" warning was not shown.
Additionally, because configuration content is rendered using <l:defer>, the form may not be present in the DOM when confirm.js initializes on window.load, preventing event listeners from being attached.
This change restores the expected behavior by including lib.form.confirm in the new layout and ensuring initialization occurs once the configuration form is available.

Testing done

1. Enabled the new Manage Jenkins UI via experimental flag.
2. Modified configuration values in:
   • Manage Jenkins → Configure System
   • Manage Jenkins → Security
   • Manage Jenkins → Appearance
3. Verified that navigating away after modifying a field shows the native browser "unsaved changes" warning.
4. Verified that clicking Save clears the dirty state and does not trigger the warning.
5. Verified that classic configuration pages (e.g., /configure) still show the warning as expected.
6. Confirmed no new console errors were introduced.
   Build and verification:

• mvn -pl war -am verify -P light-test -Dskip.yarn
• yarn lint
• mvn spotless:check

Screenshots (UI changes only)

Before

Modified a configuration checkbox and navigated away — no warning dialog appeared.

After

Modified a configuration checkbox and navigated away — native browser unsaved changes warning is shown.

Proposed changelog entries

• Restore unsaved changes warning in the new Manage Jenkins UI.

Proposed changelog category

/label bug

Proposed upgrade guidelines

N/A

Submitter checklist

• The issue, if it exists, is well-described.
• The changelog entries and upgrade guidelines are appropriate.
• There is automated testing or an explanation as to why this change has no tests.
• UI changes do not introduce CSP regressions.

Desired reviewers

@jenkinsci/core-pr-reviewers

Maintainer checklist

• There are at least two (2) approvals for the pull request and no outstanding requests for change.
• Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
• Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
• Proper changelog labels are set so that the changelog can be generated automatically.
• If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
• If it would make sense to backport the change to LTS, be a Bug or Improvement, and either the issue or pull request must be labeled as lts-candidate to be considered.

[welcome]

Yay, your first pull request towards Jenkins core was created successfully! Thank you so much!

A contributor will provide feedback soon. Meanwhile, you can join the chats and community forums to connect with other Jenkins users, developers, and maintainers.

[mawinter69]

Without testing myself I assume that the delayed loading in js is sufficient already. If there is a page where it isn't working then I assume it is also not working with the new manage page turned off. So might be good if you can test all pages that use l:settings-subpage. If that is the case then adding the adjunct to that page would be the proper approach I think.
It might also be a valid approach to generally include that unconditionally on l:settings-subpage, then we could remove the adjuncts from all jelly files that use l:settings-subpage.

[mawinter69]

I think it is not 100% correct to include that here. You will find that the pages that use <l:settings-subpage usually already include the adjunct. And frequently conditionally requiring permissions. So when you are in readonly mode you can't modify anything. Please test whether all pages still properly react on page leave when you remove this.

[Heller07]

I tried removing <st:adjunct includes="lib.form.confirm"/> from settings-subpage.jelly and tested the pages using <l:settings-subpage> (Configure System, Security, Appearance).
After removing it, the unsaved changes warning no longer appears in the new Manage Jenkins UI.
With the adjunct included at the layout level, the warning behaves as expected.
From my testing it seems that lib.form.confirm is not consistently included by all pages rendered via <l:settings-subpage>, so including it here ensures consistent dirty-form tracking.

[mawinter69]

the adjunct is included in the global config page, but I think due to the deferred loading, the load event has already happened and so the js can't hook into the change events in the form.
I added an load event listener in hudson-behavior.js that logs when the load event was called and to confirm.js when the js was loaded. This is what I get:

20:54:49.096 hudson-behavior.js:2751 Window load event called.
...
20:54:57.853 confirm.js:149 confirm.js was loaded

So there are almost 8 seconds between the load event until the confirm.js is loaded.

By adding it generally at the beginning the confirm.js is loaded earlier and thus can hook in properly. That probably means any page using <l:settings-subpage doesn't need to include the adjunct explicitly or indirectly. Though it doesn't hurt to include it twice as stapler will take care not to double load it.

The CloudSet/index.jelly uses noDefer and there I can see that the load event reaches the confirm.js. But then there is nothing to hook in it seems.

[Heller07]

Thanks for adding the logging, that makes the timing issue much clearer.
From the logs it looks like confirm.js is loaded several seconds after the window.load event has already fired when using <l:defer>, so its load listener never runs and it cannot attach the change listeners.
Including the adjunct earlier causes confirm.js to load before the load event and therefore work as expected.
That said, it might be cleaner to make confirm.js resilient to late loading instead of relying on earlier inclusion. I can try updating it to initialize immediately if the document is already loaded. Let me know if that would be preferable.

[mawinter69]

I think confirm.js should be loaded as late as possible to ensure when it loads that it detects all possible things that are changeable. So it should be made resilient to late loading.

[Heller07]

I’ve updated confirm.js to initialize immediately if the document is already loaded, and to observe DOM changes so it attaches once the configuration form becomes available.
This keeps the script resilient to late loading without requiring earlier inclusion. Please let me know if you’d like further adjustments.

[Heller07]

Hi @mawinter69 , just checking in on this PR.
Is there anything I should update or adjust based on the discussion so far?

[mawinter69]

Small change otherwise looks good. Tested that and I can confirm that changes to the configure page are properly detected.

[mawinter69]

Revert that

[Heller07]

Thanks for the review and for testing. I’ve applied the requested change.

[timja]

LGTM - tested locally.

[mawinter69]

LGTM

[Heller07]

Hi @timja I see that some tests which were previously passing are now failing. Does this need a re-run, or is there something else that requires attention from my side?

[MarkEWaite]

Hi @timja I see that some tests which were previously passing are now failing. Does this need a re-run, or is there something else that requires attention from my side?

The failing test has been disabled by pull request:

• Disable testRemoteUpdateSitePerformingValidation temporarily #26342

It will be fixed with issue:

• testRemoteUpdateSitePerformingValidation fails on master branch #26341

You can disable the failing test by updating the branch or wait a little longer for the fix.

[timja]

/label ready-for-merge

---

This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

[Heller07]

The related issue appears to be fixed now. A CI re-run would be helpful.

Hi @timja I see that some tests which were previously passing are now failing. Does this need a re-run, or is there something else that requires attention from my side?

The failing test has been disabled by pull request:

* [Disable testRemoteUpdateSitePerformingValidation temporarily #26342](https://github.com/jenkinsci/jenkins/pull/26342)

It will be fixed with issue:

* [testRemoteUpdateSitePerformingValidation fails on master branch #26341](https://github.com/jenkinsci/jenkins/issues/26341)

You can disable the failing test by updating the branch or wait a little longer for the fix.

The related issue appears to be fixed now. A CI re-run would be helpful.

[welcome]

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.