Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-60866][JENKINS-71513] Adapt to upcoming st:bind change #830

Merged
merged 3 commits into from
Feb 27, 2024

Conversation

daniel-beck
Copy link
Member

@daniel-beck daniel-beck commented Sep 28, 2023

Related to jenkinsci/stapler#385 and jenkinsci/jenkins#6865.

This is not strictly required, as Stapler/Jenkins will continue to work as before even with the existing code, but this PR demonstrates the change in behavior (without this change, it's the backward compatibility mode, with this change it's the new behavior).

Currently this tag emits something like the following when running on the core PR build:

<script>window.bindings['buildMonitor']=makeStaplerProxy('/jenkins/$stapler/bound/<UUID>','<CSRF_CRUMB>',['fetchJobViews']);</script>

With this change, on the Jenkins PR build, it's instead

<script src="/jenkins/$stapler/bound/script/jenkins/$stapler/bound/<UUID>?var=buildMonitorBind&amp;methods=fetchJobViews" type="application/javascript"></script>

(Behavior changes based on whether var is a legal identifier or not, and it's not without the PR.)

This does not result in the rest of this plugin (or even this file) becoming compatible with https://plugins.jenkins.io/csp/ or whatever form that'll take in Jenkins core in the future, but that's beyond the scope of this PR.

Testing done

Opened a BM view and the Stapler proxy calls happened periodically, both with and without this change, when running the core PR incremental build.

Submitter checklist

@basil basil merged commit d06ef25 into jenkinsci:master Feb 27, 2024
15 checks passed
@basil basil added the internal label Feb 27, 2024
<script>
window.bindings['buildMonitor'] = buildMonitorBind
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't there be a semicolon at the end of this statement?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably safer to have it, not strictly necessary though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants