preventDefault() on form.dispatchEvent(new Event('submit'))?

RESOLVED FIXED in Firefox 56

Status

()

defect
P3
normal
RESOLVED FIXED
2 years ago
7 hours ago

People

(Reporter: janx, Assigned: edgar)

Tracking

(Blocks 1 bug)

unspecified
mozilla69
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed, firefox69 fixed)

Details

()

Attachments

(2 attachments, 1 obsolete attachment)

Reporter

Description

2 years ago
When pasting the following address into various browsers, we get different results:

data:text/html,
<form action="https://www.mozilla.org/" id="form">
  <input type="submit" value="Navigate maybe">
</form>
<script>
  var form = document.getElementById("form");
  form.addEventListener("submit", function (event) {
    console.log(event.cancelable);
    event.preventDefault();
  });
  form.dispatchEvent(new Event('submit'));
</script>

- Firefox will navigate away (`event.preventDefault()` won't work)
- Chrome will not navigate away (`event.preventDefault()` will have the last word)

The correct behavior according to specs is to navigate (like Firefox does), because `new Event('submit')` is not cancelable unless you specify `new Event('submit', { cancelable: true })`.

I'm not sure if Chrome's behavior is a bug, or if it's intended to be helpful (my code does look like I'm trying hard not to submit), so I'll file a Chrome bug as well.

See also: https://stackoverflow.com/questions/40916300/how-can-i-preventdefault-on-dispatchevent
Thanks for the bug! It would be useful to know if this breaks sites for us to help us decide what to do here (i.e., implement Chrome's behavior and update the spec, or just close as INVALID).


I added the "hotlist-interop" label to the Chromium bug.
Component: General → DOM: Events
Product: Web Compatibility → Core
Stone may have thoughts here. Otherwise we can wait for Anne to return from PTO.
Flags: needinfo?(sshih)
Flags: needinfo?(annevk)
Like Janx's description, FF follows the spec and Chrome's behavior is helpful to web authors. Considering to be helpful to web authors, maybe we could consider giving the corresponding values of bubbles and cancelable for each kind of event so that users could simply create an event and dispatch it without caring the bubbles and cancelable values. But I don't know if it's worth to do it.
Flags: needinfo?(sshih)
Priority: -- → P3
Reporter

Comment 4

2 years ago
FYI, the Chromium bug was closed as WONTFIX:

From https://bugs.chromium.org/p/chromium/issues/detail?id=730108#c4
> This is a bug of Firefox. Dispatching non-trusted 'submit' event should not start form submission.  The specification doesn't define such behavior AFAIK.
Reporter

Comment 5

2 years ago
FYI Safari and Edge don't seem to navigate away either (Chrome's behavior).

Firefox seems to be the only browser that navigates away on the example code.
According to [1], untrusted events shouldn't trigger the UA default actions.

[1] https://www.w3.org/TR/uievents/#trusted-events
Assignee: nobody → sshih

Comment 8

2 years ago
To close my needinfo: Ming-Chou is correct (although note that there's an exception for the click event, as defined by https://dom.spec.whatwg.org/ in the Events section).
Flags: needinfo?(annevk)
Attachment #8879069 - Flags: review?(bugs)
Comment on attachment 8879069 [details] [diff] [review]
Bug 1370630 - Untrusted submit event shouldn't trigger form submission

Need to change also HTMLFormElement::GetEventTargetParent and maybe also HTMLFormElement::WillHandleEvent
Attachment #8879069 - Flags: review?(bugs) → review-
(In reply to Olli Pettay [:smaug] from comment #9)
> Comment on attachment 8879069 [details] [diff] [review]
> Bug 1370630 - Untrusted submit event shouldn't trigger form submission
> 
> Need to change also HTMLFormElement::GetEventTargetParent and maybe also
> HTMLFormElement::WillHandleEvent
I didn't change WillHandleEvent because it's related to the event propagation of nested form elements and I don't see a reason to have different behaviors of trusted or untrusted events.
Attachment #8883489 - Flags: review?(bugs)
Attachment #8883489 - Flags: review?(bugs) → review+

Comment 13

2 years ago
Pushed by sshih@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/367b6f947f87
Untrusted submit event shouldn't trigger form submission. r=smaug.

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/367b6f947f87
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
So, I have a mozilla-internal jenkins site (that uses YUI), that has been broken by this. I used mozregression to bisect the site problem back to exactly this change. 

Unfortunately, that site is not available to mozilla employees in general, so not sure how to give you something you can debug. Although, perhaps since this change might have impact on sites, this may be just an example of one (Jenkins).
reopened & marked as regression to catch folks eyes sooner.
Status: RESOLVED → REOPENED
Has Regression Range: --- → yes
Keywords: regression
Resolution: FIXED → ---
Note: the site does work Chrome Version 59.0.3071.115 on macOS 10.12.5
This one is fixed. Please file a followup bug or ask this one to be backed out (and once backed out, reopen).
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago2 years ago
Resolution: --- → FIXED
John, do you know why the site works on Chrome? I assume Jenkins uses some Gecko specific code it shoudn't use.
Flags: needinfo?(jrgm)
Mozregression shows this bug breaks https://webcompat.com/issues/8128 or any jenkins login, any idea?

Comment 21

2 years ago
Backout by sshih@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c7c8b33ba324
Backed out changeset 367b6f947f87 for breaking mozilla-internal jenkins site. r=backout.

Comment 22

2 years ago
Added an URL of a public Jenkins server
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 23

2 years ago
backoutbugherder
https://hg.mozilla.org/mozilla-central/rev/c7c8b33ba324
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago2 years ago
Resolution: --- → FIXED
reopen again
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Change Chrome's user agent to be the same as Firefox, Chrome doesn't work anymore. Apply my patch and change the user agent to be the same as Chrome, button submission works.
(In reply to Ming-Chou Shih [:stone] from comment #25)
> Change Chrome's user agent to be the same as Firefox, Chrome doesn't work
> anymore. Apply my patch and change the user agent to be the same as Chrome,
> button submission works.

Hi Eric,
According to the testing results, it's because the website behaves differently for different user agents and I have no idea how to proceed. Any suggestions about it?
Flags: needinfo?(etsai)
hi :stone, could you help to update some more detail about different behavior from which website? the public Jenkins server or mine private one?
Flags: needinfo?(etsai)
I verified on http://jenkins2.legacyserver.in/

I can verify it on the public server later.
(In reply to Ming-Chou Shih [:stone] from comment #28)
> I verified on http://jenkins2.legacyserver.in/
> 
> I can verify it on the public server later.
Oops. I don't have an account nor find a link to sign up.
(In reply to Ming-Chou Shih [:stone] from comment #29)
> (In reply to Ming-Chou Shih [:stone] from comment #28)
> > I verified on http://jenkins2.legacyserver.in/
> > 
> > I can verify it on the public server later.
> Oops. I don't have an account nor find a link to sign up.

Hi Eric,
I only can confirm that the problem on [1] is caused by the user agent. I don't have access to the public jenkins website [2]. Any suggestions?

[1] http://jenkins2.legacyserver.in/
[2] https://jenkins.qa.ubuntu.com/login?from=%2F
Flags: needinfo?(etsai)
Per-talk with :stone, I think if it's solved in an internal jenkins server, we should land first to make sure the behavior is correct.
If there is any UA detection makes this not work, please file another bug in webcompat general or Tech_Evangelism to outreach.
Flags: needinfo?(etsai)
See Also: → 1399783
Assignee

Updated

10 months ago
See Also: → 1477286
Assignee

Updated

10 months ago
Depends on: 1399783
Keywords: regression
See Also: 1399783

Comment 32

7 months ago
https://github.com/jenkinsci/jenkins/pull/3689 is a PR to make Jenkins work again as soon the fix has landed in FF, however that means the server needs to use the newest version of jenkins or patch their version of hudson-behavior.js as demonstrated in the pr.
Assignee

Updated

5 months ago
Duplicate of this bug: 1477286
Assignee

Updated

5 months ago
Blocks: 1487705
Assignee

Updated

5 months ago
Assignee: stone123456 → echen

:stone, would you mind updating the patch? As https://issues.jenkins-ci.org/browse/JENKINS-53462 states the depending Jenkins issue will be fixed soon. As such it would be great to have a build for testing the fix. Thanks!

Flags: needinfo?(stone123456)
Assignee

Comment 36

a month ago

:stone is no longer active. I have rebased the patch and also triggered a try run, https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f12e17b558a36ef5b65235d663c4e617c3d328e

Flags: needinfo?(stone123456)

Thanks Edgar! I will forward that link to the Jenkins people for testing.

So the Jenkins patch was tested with the above try build of Firefox, and they were able to confirm that their fix works with the patch on this bug. So it will go live in Jenkins 2.174. When the LTS release will get the fix isn't clear yet. But here the comment from one of the maintainers:

https://issues.jenkins-ci.org/browse/JENKINS-53462?focusedCommentId=364824&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-364824

2.164.3, scheduled for May 8, the earliest, if accepted as a backport. The next .1, scheduled for early June, otherwise. Assuming of course we don't cause regressions again.

As such we might have to wait until early June at latest before we can push this patch. I will keep an eye on the releases, and will let you know.

Actually including this patch in Firefox in the near future might not be easy given the following reply from Daniel Beck about the usage statistics of Jenkins LTS versions:

https://issues.jenkins-ci.org/browse/JENKINS-53462?focusedCommentId=365324&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-365324

Sylvestre, what is the take from Release managers in how best we can get this fixed without annoying users of Jenkins?

Flags: needinfo?(sledru)
Target Milestone: mozilla56 → ---

We would need to get the patch in nightly.

Depends when Edgar can get it landed but it will be probably too late for 67

Flags: needinfo?(sledru)

(In reply to Sylvestre Ledru [:sylvestre] from comment #40)

We would need to get the patch in nightly.

That for sure...

Depends when Edgar can get it landed but it will be probably too late for 67

No, we don't clearly want it for 67 and maybe also not for 68, whereby this would be great. Please see the link from my last comment in why this is a bit more complicated to land without affecting Jenkins users. If we don't care we could land after the next merge. Then there are still 12 weeks left before we reach release, but some users might not have updated their Jenkins installation by then. From my experience I can say it's not always easy and takes time.

So landing this patch for Firefox 69 affected Jenkins folks could still use our upcoming Firefox 68 ESR release with older versions of Jenkins. Maybe that could be a compromise?

Flags: needinfo?(sledru)

Forwarding needinfo for the last comment from Sylvestre to Mike.

Flags: needinfo?(sledru) → needinfo?(miket)

So landing this patch for Firefox 69 affected Jenkins folks could still use our upcoming Firefox 68 ESR release with older versions of Jenkins. Maybe that could be a compromise?

That sounds reasonable to me.

Flags: needinfo?(miket)

Great! So lets wait for May 14th or 15th when the next merge has been done. I will set a reminder for it.

Flags: needinfo?(jrgm)

FYI a new Jenkins LTS got released 2 days ago:
https://jenkins.io/changelog-stable/#v2.164.3

And it contains:

Make form submit buttons on the Jenkins classic UI compatible with potentially upcoming Firefox bug fix. (issue 53462, Firefox bug 1370630)

Hopefully we will be able to land this patch after our next merge. I will continue to monitor.

After 14 days there are still no reports for Jenkins which claim that their patch has been caused any kind of problems. As such do we want to move forward on this bug and get the patch landed for Firefox 69 only?

Flags: needinfo?(echen)
Assignee

Comment 47

22 hours ago

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #46)

After 14 days there are still no reports for Jenkins which claim that their patch has been caused any kind of problems. As such do we want to move forward on this bug and get the patch landed for Firefox 69 only?

Yes, lets land this patch for 69. Thanks.

Flags: needinfo?(echen)

Comment 48

14 hours ago
Pushed by echen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ab9d5eeaa90e
Untrusted submit event shouldn't trigger form submission; r=smaug

Comment 49

7 hours ago
bugherder
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago7 hours ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
You need to log in before you can comment on or make changes to this bug.