Open Bug 1370630 Opened 4 years ago Updated 16 days ago

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

Categories

(Core :: DOM: Forms, defect, P3)

defect

Tracking

()

REOPENED
Tracking Status
relnote-firefox --- -

People

(Reporter: janx, Assigned: edgar)

References

()

Details

(Keywords: dev-doc-needed, regression, site-compat)

Attachments

(2 files, 1 obsolete file)

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
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.
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
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+
Pushed by sshih@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/367b6f947f87
Untrusted submit event shouldn't trigger form submission. r=smaug.
https://hg.mozilla.org/mozilla-central/rev/367b6f947f87
Status: NEW → RESOLVED
Closed: 3 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
Closed: 3 years ago3 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?
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.
Added an URL of a public Jenkins server
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/c7c8b33ba324
Status: REOPENED → RESOLVED
Closed: 3 years ago3 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
See Also: → 1477286
Depends on: 1399783
Keywords: regression
See Also: 1399783
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.
Duplicate of this bug: 1477286
Blocks: 1487705
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)

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

(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)
Pushed by echen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ab9d5eeaa90e
Untrusted submit event shouldn't trigger form submission; r=smaug
Status: REOPENED → RESOLVED
Closed: 3 years ago2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
Duplicate of this bug: 1487705

Edgar, is that a change we actually should put into the developer release notes for the 69 release?

Flags: needinfo?(echen)

Release Note Request (optional, but appreciated)
[Why is this notable]: Form submission now won't be triggered by un-trusted event which is generated from web page, some web page might rely on this special behavior of Firefox, like bug 1399783. It's better to put into release notes.
[Affects Firefox for Android]: Yes
[Suggested wording]:
[Links (documentation, blog post, etc)]:

relnote-firefox: --- → ?
Flags: needinfo?(echen)
Regressions: 1556980

That's a developer note that is needed, so MDN works. Adjusting flags accordingly.

Keywords: site-compat

Is there any effective difference between patch landed here and that in bug 1535988?

The patch in bug 1535988 caused bug 1537555 and thus was reverted. Would this change cause the exactly the same issue again?

And it seems that the answer is yes. Bug 1556980 is just the new bug 1537555 :/

Regressions: 1564080

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #54)

Is there any effective difference between patch landed here and that in bug 1535988?

The patch in bug 1535988 caused bug 1537555 and thus was reverted. Would this change cause the exactly the same issue again?

Right. Need to back out.

ni? Edgar re: Comment #57

Flags: needinfo?(echen)

Actually, it seems that Edgar is on leave. Oops.

Olli, can you help w/ the backout?

Flags: needinfo?(echen) → needinfo?(bugs)

Oh wait, I can just ask a Sheriff. Sorry for the mailspam.

Flags: needinfo?(bugs)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla69 → ---

Ryan, how do I request uplift for a backout changeset? The original patch landed in 69, but the backout happened in 70.

Flags: needinfo?(ryanvm)

Yeah, let's get the backout on Beta too before we build 69.0b4.

Flags: needinfo?(ryanvm) → needinfo?(aryx.bugmail)
Duplicate of this bug: 1535988

So do I understand correctly that landing of this patch is blocked by pages still using YUI2 which is deprecated since 2011? Or is there something more?

Depends on: 1537555
Severity: normal → S3
Component: DOM: Events → DOM: Forms

Hi Dennis, we would like to remove this firefox-only behaviour, but Jenkins does some UA detection and does different things, idealy they should just do the same thing as other browsers. Per https://github.com/jenkinsci/jenkins/pull/3689, it seems Jenkins already landed the issue, but https://issues.jenkins-ci.org/browse/JENKINS-53462 was reopen? Could someone from webcompat team help to check the status of Jenkins? Thanks

In the mean time, I will try to add Telemetry to see the usage of this.

Flags: needinfo?(dschubert)

This is probably something for Henrik, as he was in touch with the Jenkins folk in their issue tracker. Henrik, can you ping them again? :)

Flags: needinfo?(dschubert) → needinfo?(hskupin)

I asked on the Jenkins issue. Will report back when I get an update.

Flags: needinfo?(hskupin)

(In reply to Edgar Chen [:edgar] from comment #67)

Hi Dennis, we would like to remove this firefox-only behaviour, but Jenkins does some UA detection and does different things, idealy they should

So what about the YUI 2.x issues that were detected in comment 54 and following? As a result of that your patch on this bug has been backed out from 69 beta. Did anything change here in the last year?

Flags: needinfo?(echen)
Flags: needinfo?(bugs)

Edgar it would be great to get your feedback here. The Selenium project cannot make the change without that bug fixed first.

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

So what about the YUI 2.x issues that were detected in comment 54 and following?

Yes, the issue still there, not sure how YUI3 handle this.

As a result of that your patch on this bug has been backed out from 69 beta. Did anything change here in the last year?

No, just try to make some progress, what I plan to do is

  • Land the fix, but behind a pref and off by default.
  • Add Telemetry to see the usage of this.
  • Maybe show some warnning on console saying this is deprecated.
Flags: needinfo?(echen)
Flags: needinfo?(bugs)

That sounds fine. I can reach out to Jenkins again whenever we know that we want to turn this feature on by default. Note that the best time might be again after the next major ESR release whatever this will be?

You need to log in before you can comment on or make changes to this bug.