Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion core/src/main/resources/lib/layout/copyButton/copyButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ Behaviour.specify(
0,
function (copyButton) {
copyButton.addEventListener("click", () => {
if (isSecureContext) {
// HTMLUnit 2.70.0 does not recognize isSecureContext
// https://issues.jenkins.io/browse/JENKINS-70895
if (!window.isRunAsTest && isSecureContext) {
Comment on lines +7 to +9
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't

Suggested change
// HTMLUnit 2.70.0 does not recognize isSecureContext
// https://issues.jenkins.io/browse/JENKINS-70895
if (!window.isRunAsTest && isSecureContext) {
if (!window.isSecureContext) {

be the simpler change (found in @jtnord's Jira comment)?

Copy link
Contributor Author

@MarkEWaite MarkEWaite Mar 31, 2023

Choose a reason for hiding this comment

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

I see documentation on MDN for the global isSecureContext. In the MDN documentation for window, the window.isSecureContext heading links to the documentation of the global property. I assumed that means the MDN recommends use of the global rather than using window.isSecureContext.

There is not a window.isSecureContext page on MDN (404 from https://developer.mozilla.org/en-US/docs/Web/API/Window/isSecureContext ), though there was at one point as indicated by a link from a stackoverflow article.

I also believe that I read a recommendation on some location that the global isSecureContext was preferred. However, I can't find that reference now.

I also prefer the added check of window.isRunAsTest because it makes it clear that this workaround can be removed when we update to an HTMLUnit version that recognizes the global isSecureContext. See the response from the maintainer of HTMLUnit in:

If changing the implementation to the window.secureContext instead of !window.isRunAsTest && isSecureConext will allow this to be merged, then I am happy to change it to that form.

The W3C Candidate Recommendation Draft, 18 September 2021 spec for secure context says in section 2.2.2:

An application can determine whether it’s executing in a secure context by checking the isSecureContext boolean defined on WindowOrWorkerGlobalScope.

Since the spec says that the boolean is defined on window or worker global scope, I believe we can use either of them.

Copy link
Member

Choose a reason for hiding this comment

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

Fine for me either way, just seemed simpler.

// Make an invisible textarea element containing the text
const fakeInput = document.createElement("textarea");
fakeInput.value = copyButton.getAttribute("text");
Expand Down