[JENKINS-69658] - CSP compatibility for summary.jelly

[shlomomdahan]

JENKINS-69658

This PR is an extension of #451 to fix the inline JS in the summary.jelly file.

The failureSummary.js file had to be changed so that it can handle event delegation as some of the elements we are targeting with the fix do not actually appear in the html until the parent is expanded.

Testing done

• STATUS QUO: No changes implemented. This is the intended functionality.

https://www.loom.com/share/796f46d313e34c8cbbaf85963c1ffb59?sid=20dc0f68-ddca-4f88-a977-1580816dcdab

• NON RESTRICTIVE: This is the behavior after implementing the CSP changes. The code is working just as before and there is no regression in functionality.

https://www.loom.com/share/f7a8117e45a54da5a719599f0d1adb08?sid=8c812aef-9c9a-4a81-b662-c49229d68dbf

• RESTRICTIVE: This is with CSP restrictive mode enabled. It is working correctly as intended without any issues.

https://www.loom.com/share/6e3642baf5934fe59e2efdc3f3f0ec3b

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

[timja]

please use addEventListener like the code removed was using, see https://stackoverflow.com/questions/6348494/addeventlistener-vs-onclick

[shlomomdahan]

Sure - I just made this correction in the latest commit.

[daniel-beck]

(Disclaimer: Still haven't run this.) This change request looks wrong since unlike the earlier code, the events are re-initialized on HTTP response without removing previous event listeners? What happens after a few showFailureSummary, does that collect event listeners?

[shlomomdahan]

Hi @daniel-beck

If I understand correctly, this shouldn't be an issue with the new change:

container.querySelectorAll('a[id$="-showlink"], a[id$="-hidelink"]').forEach(link => {
        link.addEventListener('click', handleShowHideClick);
        link.style.cursor = 'pointer';

since duplicate event listeners cannot be added to elements see docs

https://stackoverflow.com/questions/10364298/what-does-calling-addeventlister-twice-do

Is this what you were referring to?

Thanks

[daniel-beck]

Ah, good point, I had only looked at the previous reference. So this won't cause trouble then. Thanks for the link!

[timja]

Thanks looks good, (untested)