[JENKINS-70895] skip isSecureContext check in automated tests

[MarkEWaite]

JENKINS-70895 skip isSecureContext check in automated tests

JENKINS-70895 reports that isSecureContext is not recognized by HTMLUnit in its JavaScript handling. That breaks tests for some plugins that use HTMLUnit to test configuration round trips.

#7665 introduced this change to notify users running over HTTP connections (insecure) that the copy button does not work over insecure connections.

JENKINS-21052 was the original motivation for that informational message to the user.

Testing done

• James Nord tested this change in the automated test that was failing with Jenkins 2.397 and confirmed that the test passes with this change.
• I tested interactively over HTTP and confirmed that the copy button on the inbound agent page still reports that copy is not supported over an insecure session.
• I tested interactively over HTTPS and confirmed that the copy button on the inbound agent page still correctly copies the expected content.

I spent several unsuccessful hours trying to create an automated test to show the failure. I think we should include this fix without an automated test because the automated tests in the plugin that James maintains will detect the issue.

Proposed changelog entries

• N/A - test fix that is not visible to users.

Proposed upgrade guidelines

N/A

Submitter checklist

• The Jira issue, if it exists, is well-described.
• The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples).
  • Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
• There is automated testing or an explanation as to why this change has no tests.
• New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
• New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
• New or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
• For dependency updates, there are links to external changelogs and, if possible, full differentials.
• For new APIs and extension points, there is a link to at least one consumer.

Desired reviewers

@jtnord

Maintainer checklist

Before the changes are marked as ready-for-merge:

• There are at least two (2) approvals for the pull request and no outstanding requests for change.
• Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
• Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
• Proper changelog labels are set so that the changelog can be generated automatically.
• If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
• If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

[jtnord]

validated the change with one of the failing plugins and mvn verify -Djenkins.version=2.398-rc33491.7d0392eefa_09 -Pconsume-incrementals

[daniel-beck]

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)?

[MarkEWaite]

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:

• isSecureContext not recognized in JavaScript HtmlUnit/htmlunit#573

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.

[daniel-beck]

Fine for me either way, just seemed simpler.

[MarkEWaite]

This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback.