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

refactor: Do not rely on prototype.js #98

Merged
merged 2 commits into from
Jul 11, 2023

Conversation

rahulsom
Copy link
Contributor

Jenkins has been using a very old version of Prototype.js. Starting with 2.406 of Jenkins, prototype is being gradually removed.

This change removes direct calls to prototype in the git-parameter plugin.

Refs: JENKINS-71298

Testing done

I've got another PR open (#97) that verifies the UI still works.
I ran the find command suggested in the ticket - and it only shows a call to jQuery left after this.

Submitter checklist

Preview Give feedback

@rahulsom rahulsom marked this pull request as draft June 26, 2023 23:09
Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

Many thanks for removing usages of Prototype from this plugin.

@rahulsom rahulsom force-pushed the JENKINS-71298 branch 2 times, most recently from af48fbf to af09467 Compare June 27, 2023 21:27
@rahulsom rahulsom marked this pull request as ready for review June 27, 2023 21:38
Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

The Prototype migration is incomplete. For example, when running with the user experimental flag to disable Prototype, I see console log errors like this:

TypeError: $ is not a function
    gitParameterUpdateSelect http://127.0.0.1/adjuncts/89a94610/net/uaznia/lukanus/hudson/plugins/gitparameter/javascript/git-parameter-select.js:7
    <anonymous> http://127.0.0.1/adjuncts/89a94610/net/uaznia/lukanus/hudson/plugins/gitparameter/javascript/git-parameter-select.js:109
    h http://127.0.0.1/static/89a94610/scripts/hudson-behavior.js:1937
    refillOnChange http://127.0.0.1/static/89a94610/scripts/hudson-behavior.js:1961
    <anonymous> http://127.0.0.1/adjuncts/89a94610/net/uaznia/lukanus/hudson/plugins/gitparameter/javascript/git-parameter-select.js:107
    applySubtree http://127.0.0.1/static/89a94610/scripts/behavior.js:135
    applySubtree http://127.0.0.1/static/89a94610/scripts/behavior.js:126
    applySubtree http://127.0.0.1/static/89a94610/scripts/behavior.js:107
    apply http://127.0.0.1/static/89a94610/scripts/behavior.js:90
    start http://127.0.0.1/static/89a94610/scripts/behavior.js:85
    onload http://127.0.0.1/static/89a94610/scripts/behavior.js:152
    onload http://127.0.0.1/static/89a94610/scripts/behavior.js:152
    addLoadEvent http://127.0.0.1/static/89a94610/scripts/behavior.js:151
    createSearchBox http://127.0.0.1/static/89a94610/scripts/hudson-behavior.js:2420
    <anonymous> http://127.0.0.1/adjuncts/89a94610/jenkins/views/JenkinsHeader/search-box.js:4
    <anonymous> http://127.0.0.1/adjuncts/89a94610/jenkins/views/JenkinsHeader/search-box.js:6

@rahulsom rahulsom marked this pull request as draft June 27, 2023 22:41
@rahulsom
Copy link
Contributor Author

Taking this back to draft so I can fix that problem. I'm looking at a few other things - should be back to this shortly.

@rahulsom
Copy link
Contributor Author

@basil Do you have ideas about how we could get the default user to have that flag?

@basil
Copy link
Member

basil commented Jun 28, 2023

UserExperimentalFlagsPropertyTest in core shows an example of how to run a test as a particular user with a particular flag set. However, RemovePrototypeUserExperimentalFlag isn't available until 2.404, which hasn't released LTS yet.

Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

Very nice so far! Just three more points and I should be ready to approve this.

Jenkins has been using a very old version of Prototype.js.
Starting with 2.406 of Jenkins, prototype is being gradually removed.

This change removes direct calls to prototype in the git-parameter plugin.

Refs: JENKINS-71298
Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

Nice job! All the changes from jenkinsci/jenkins#7982 appear to be ported correctly and interactive testing on 2.412 shows no regression in both the success and failure code paths.

@basil basil self-assigned this Jun 29, 2023
@basil basil marked this pull request as ready for review June 29, 2023 18:07
@basil
Copy link
Member

basil commented Jun 29, 2023

CC @jenkinsci/core-security-review

@basil basil requested a review from klimas7 June 29, 2023 18:08
@basil basil added the internal label Jun 29, 2023
@basil basil merged commit ce9bc7b into jenkinsci:master Jul 11, 2023
@basil
Copy link
Member

basil commented Jul 11, 2023

Released in 0.9.19.

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