[JENKINS-73900] Un-inline JS in ModuleLocation/config.jelly and fix validation logic

[shlomomdahan]

JENKINS-73900

This pull request addresses two main improvements for the Subversion plugin:

• Fixes the form validation logic for the excludedRevprop property, which was previously broken.
• Improves CSP compatibility by extracting inline JS

Testing done

Original validation logic improvement: Loom Video
With CSP restrictions enabled: Loom Video

Submitter checklist

• Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
• Ensure that the pull request title represents the desired changelog entry
• Please describe what you did
• Link to relevant issues in GitHub or Jira
• Link to relevant pull requests, esp. upstream and downstream changes
• Ensure you have provided tests - that demonstrates feature works or fixes the issue

[yaroslavafenkin]

Let me know if you get stuck or any of the dots do not connect. I could also provide a patch myself if this all is too confusing :P

[yaroslavafenkin]

Why do we need to dispatch the event here? Is that supposed to retrigger the validation for excludedRevprop field when we change either remote or credentialsId? Is that how it worked before your change?

I don't like it personally, I'd prefer to have it triggered only when excludedRevprop loses focus. But disregard my comment if it worked like this before, who am I to judge :)

[shlomomdahan]

Yes, that is the way it was "supposed" to work before. Although it wasn't working correctly.

 if (self===document.getElementById('svn.remote.loc')){
                r=findNextFormItem(self,'excludedRevprop');
                r.onchange(r);
            }

I think it makes sense as we should validate the excluded revProp whenever remote or credentials changes as well as when excluded revProp itself changes

[yaroslavafenkin]

I expect this will fall apart if we try to define a second location in the configuration, because you selectors will return arrays of elements. Could you check how it behaves in that case?

[yaroslavafenkin]

Nevermind, disregard "arrays of elements", you're using querySelector, not querySelectorAll. Either way I'm concerned whether this is going to work when we try to define multiple locations.

[shlomomdahan]

I see what you're saying.

In this case, each time you add a module, the checkUrl and checkCredentials is correctly called for the corresponding module, however, the validation for the excludedRevprop only works on the module that was added first.

We can discuss this on the call this morning

[yaroslavafenkin]

Sorted it out on a call. excludeRevProp validation previously has referenced credentialsId and remote URL or the first module that was defined. While this behavior seems wrong it is preserved here.

[basil]

Nice work taking this through to completion!