[JENKINS-53462] Stop using deprecated submit events

[daniel-beck]

See JENKINS-53462.

Previously: #4246 #3761 #3760 #3689

In some limited manual testing in Firefox and Chrome, this seems to work.

Proposed changelog entries

• Stop using deprecated untrusted submit events on modern browsers for compatibility with future Firefox releases.

Proposed upgrade guidelines

N/A

Submitter checklist

• (If applicable) Jira issue is well described
• Changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developer, depending on the change). Examples
  • Fill-in the Proposed changelog entries section only if there are breaking changes or other changes which may require extra steps from users during the upgrade
• Appropriate autotests or explanation to why this change has no tests
• For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@thomasgl-orange @halkeye

Maintainer checklist

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

• There are at least 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 does not block the change
• Changelog entries in the PR title and/or Proposed changelog entries are correct
• Proper changelog labels are set so that the changelog can be generated automatically
• If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (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).

[oleg-nenashev]

Tested it with Jenkins PR tester on Chrome and Edge. Seems to be working fine.

We may merge it in 24 hours if there is no negative feedback. Please see the merge process documentation for more information about the merge process

[thomasgl-orange]

Tested with:

• FF 87.0 with dom.forms.submit.trusted_event_only=false
• FF 87.0 with dom.forms.submit.trusted_event_only=true
• FF 66.0.3

It looks good, forms are always submitted once and only once (with some usecases involving files parameters, some none).

Whereas plain 2.288 without this PR is very broken on FF 87.0 with dom.forms.submit.trusted_event_only=true, most forms are then impossible to submit.

PS: now also tested a bit from some Windows 10 navigators (IE 11.1790.17763.0 and Edge 44.17763.831.0), all good.

[timja]

May have caused a regression in pipeline input step (or at least in the tests)

See jenkinsci/bom#511 (comment), picked up in PCT testing

[halkeye]

if its breaking tests, what version of a browser engine are the tests using? I know we had issues in ... that repo that generates all the install lists that jenkins uses (like groovy or perl or whatever). The browser engine was so old it didn't support any of the basic javascript on some sites.

[timja]

It's using html unit https://github.com/jenkinsci/bom/pull/511/checks?check_run_id=2535385275

[res0nance]

May have caused a regression in pipeline input step (or at least in the tests)

See jenkinsci/bom#511 (comment), picked up in PCT testing

Its also breaking ssh-slaves.
I tested pipeline-input-step manually and it seems to work on 2.289 so I think its just the tests.

[daniel-beck]

The browser engine was so old it didn't support any of the basic javascript on some sites.

Also https://groups.google.com/g/jenkinsci-dev/c/Wj3YHoKRDSM/m/r6NbQG8oAgAJ as another indicator that we're either on an outdated HTMLUnit, or that there's just no support for modern web stuff.

Although my change here should be backward compatible with old browsers; assuming HTMLUnit is just an "old browser" I would expect it to continue working. Perhaps there's a legitimate problem here?

[jtnord]

https://issues.jenkins.io/browse/JENKINS-65585 FTR I tried bumping HTMLUnit in the jenkins-test-harness-htmlunit and then updating the dependency but that did not resolve the issue.

[thomasgl-orange]

I've also tried running the failing ssh-slaves test today (on jenkins.version=2.289) with htmlunit 2.49.1 (instead of 2.43.0), using a local -SNAPSHOT build of jenkins-test-harness-htmlunit, but it didn't solve this issue.

The tested form has two submit inputs, named "Yes" and "No". With Jenkins 2.289, in the HtmlUnit test, when Yes is clicked, their is no Yes parameter in the submitted request.

I think the root cause is a combination of this and that:

• https://github.com/daniel-beck/jenkins/blob/8ee48ec12a8d9f22ffe27a887b6381273651102c/war/src/main/webapp/scripts/yui/button/button-debug.js#L2703

• https://github.com/HtmlUnit/htmlunit/blob/ab68c05d7c5b7e10b49bc540c87d6a1230b3d90b/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/html/HTMLFormElement.java#L347

We are, indeed, calling requestSubmit() with no "submitter" parameter, and getting the documented behavior from HmlUnit: the form is submitted, but we don't know how/why (no Yes or No parameter).
Whereas it seems to be okay (and a bit magical?) in real browsers so far; I've tested again this specific case in FF 88, both with dom.forms.submit.trusted_event_only=false and true, and I always get the expected Yes=Yes in the request.

@daniel-beck: Do you happen to remember what was "not working" when passing a parameter to requestSubmit(...)?

[daniel-beck]

Do you happen to remember what was "not working" when passing a parameter to requestSubmit(...)?

Not specifically. It was something obvious, like an exception, so should be straightforward to reproduce. Might have been that I tried to pass the wrong kind of object? Since it worked without, I just documented the fact and moved on.

[thomasgl-orange]

It was something obvious, like an exception, so should be straightforward to reproduce. Might have been that I tried to pass the wrong kind of object?

I've tried a few things with this "submitter" parameter, that's where I'm at: #5479

[EdgarChen]

The tested form has two submit inputs, named "Yes" and "No". With Jenkins 2.289, in the HtmlUnit test, when Yes is clicked, their is no Yes parameter in the submitted request.

I guess you runs into the same issue as reported in https://bugzilla.mozilla.org/show_bug.cgi?id=1653625.

[MarkEWaite]

I'm seeing an unexpected failure during interactive use that seems to be caused by this change. I've not been able to duplicate it in a simplified case yet, but I can see it consistently with this change included and it is resolved when this change is removed.

When I attempt to configure a matrix job in Jenkins 2.288 I am able to save the configuration.

When I attempt to save the same job with Jenkins 2.289, the page does not return.

When I save the same job with a custom build of Jenkins 2.289 with this change reverted, it saves as expected.

[thomasgl-orange]

@MarkEWaite: I can reproduce what you're describing, and yes, it is indeed a consequence of this 2.289 change. It's not specific to Matrix jobs though.

What triggers the issue here is the invalid depth=0 value in your Git CloneOption. Because the shallow option is off, the field is hidden in the configure page (and the invalid value shouldn't matter in practice), but it still gets somehow validated and blocks submitting the form. And of course the error message is hidden too...

It is something which could be fixed / worked-around in the Git plugin itself, for this specific case. But similar issues might pop up with other plugins and existing configs, so it's difficult to estimate how "bad" this is.