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

prototype js removal code changes #156

Closed
wants to merge 3 commits into from

Conversation

rsahoo7495
Copy link

@rsahoo7495 rsahoo7495 commented Oct 12, 2023

changed YAHOO.lang.escapeHTML() method to javascript method in js file https://github.com/jenkinsci/matrix-auth-plugin/blob/master/src/main/resources/hudson/security/table.js

Testing done

mvn verify and mvn clean install got success
After changes and build, added plugin hpi to jenkins instance

image

provided global access to the user for read

image

provided project level access for build and read

image

Now we can have access to in the project for build

image

Debugging in browser with code, it still works well with new code

image

image

image

Submitter checklist

Preview Give feedback

@daniel-beck
Copy link
Member

Interesting approach to sanitize the string with the text node. I did it with basic escaping in #152, but this seems more elegant.

The testing done is unconvincing, it does not actually invoke the code in a manner demonstrating that escaping is still being done correctly.

@rsahoo7495 rsahoo7495 marked this pull request as ready for review October 12, 2023 13:37
@rsahoo7495
Copy link
Author

rsahoo7495 commented Oct 12, 2023

@daniel-beck thanks for your comment, escaping which u did in in #152 looks more convenient to me. So used the same logic here

@rsahoo7495
Copy link
Author

did testing with input <>"'&Raj and added debugging screenshot

@daniel-beck
Copy link
Member

In that case I'll close this PR as it doesn't offer anything over the cleanup in #152. Note that I removed Prototype in #140, this just removes uses of YUI. The plugin is compatible with Jenkins 2.426+ already.

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

Successfully merging this pull request may close these issues.

3 participants