139798 - form submit with javascript action unable to submit

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[Tony Tovar]

Using build 2002-04-22(08) on WinNT

Clicking the GO button (or simply typing a product search and pressing ENTER)
has no effect.  It works in IE.  Tech evangelism?

Comment 1

[Boris Zbarsky [:bzbarsky]]

To jkeiser.. The "action" is a javascript url that calls .submit() and I suspect
that we ignore .submit() calls on a form that is "already submitting", which
breaks in this case.

Comment 2

[John Keiser (jkeiser)]

It has an effect for me ... strange, when I type "hi" and click "GO" it goes to
a script called EMAILOPTIN, it doesn't search at all.

Comment 3

[John Keiser (jkeiser)]

Linux 2002042210.

Comment 4

[John Keiser (jkeiser)]

Oops, that was the wrong box.  They have a really sneaky box at the bottom for
you to enter your email that doesn't tell you what the box is for.  I am seeing
the problem on product search.

This is ugly code.  Maybe IE handles it by only disallowing multiple submits to
http.

Comment 5

[John Keiser (jkeiser)]

*** Bug 139840 has been marked as a duplicate of this bug. ***

Comment 6

[John Keiser (jkeiser)]

Nominating.  This is important, bz tells me there are two other bugs out there
(I haven't see them) that have this problem.  Any form which has javascript: as
the action and modifies the ADT: if this

The fix is known, and should be quick.  Simply do not block other submits when
the URL is javascript.

Comment 7

[John Keiser (jkeiser)]

Attachment: Patch

This fixes the problem by allowing submission to occur again during submit of
javascript: URLs.  I do not think this is a hack, though one thing that would
be nice is if you could ask for info about a URI--i.e. find out if it is
synchronous and find out if it is scriptable and if both are true, disable
submit.  This would work gracefully with future as-yet-undetermined URL types.

Comment 8

[John Keiser (jkeiser)]

Alex, could you r=?

Comment 9

[Alexandru Savulov]

Comment on attachment 80922 [details] [diff] [review]
Patch

r= alexsavulov

i bet the guy that put that page together tried to show everybody what nice
tricks he can do with HTML and javascript. but anyway, he showed us a problem.
so if i get it right in this case we submit only once right? now i start to
think about some sick dude that might try to submit the same form
twice(explicitly) using for example a js onsubmit handler or like here a js
action. that sould be tested with IE to see if is possible. If yes, sooner or
later someone will do it (or maybe not).

Comment 10

[John Keiser (jkeiser)]

1.0 should only be set by nsbeta1+'er.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Note: this breaks Dell's Premier Support web site....

Comment 12

[Kevin McCluskey (gone)]

nsbeta1+

Comment 13

[Marc Attinasi]

Comment on attachment 80922 [details] [diff] [review]
Patch

SR=attinasi

As we discussed in IM, please change the comment to make it clear that
submitting from JS is not really 'submitting', it is just executing some JS
code.

Comment 14

[John Keiser (jkeiser)]

Attachment: Patch v1.0.1

Final version (so that I have the right patch to go with when checking in to
branch)

Comment 15

[John Keiser (jkeiser)]

Comment on attachment 81793 [details] [diff] [review]
Patch v1.0.1

Carrying r/sr.

Comment 16

[John Keiser (jkeiser)]

Fix checked in to trunk.

Comment 17

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 81793 [details] [diff] [review]
Patch v1.0.1

a=[e-mail] for drivers

Comment 18

[Tony Tovar]

Is it too late to update the summary to something more technical?  I'm the
reporter and I'm open to any suggestions.

Comment 19

[Jaime Rodriguez, Jr.]

This sounds like it is a rare use of "HTML and javascript." Do we know how often
this occurs out in the wild? Does this happen on a high level, in any top sites?

Comment 20

[Boris Zbarsky [:bzbarsky]]

In the week or so that we have had this bug, we have had at least four reports
on it.  One of the affected sites is Dell's support site, another is a banking
site.  The fix is very safe and simple.

Comment 21

[Jaime Rodriguez, Jr.]

adt1.0.0+ (on ADT's behalf) approval for checkin to the 1.0 brnach. Pls check
this in after you receive a= from Drivers, then add the fixed1.0.0 keyword.

Comment 22

[Michael Dunn]

[Internal reference testcase]
http://bugscape.mcom.com/show_bug.cgi?id=14642

Comment 23

[chris hofmann]

did this one make it to the branch yet?

Comment 24

[Randell Jesup [:jesup] (needinfo me)]

Refresh of a= for drivers; very low risk, high reward/visibility

Comment 25

[scottputterman]

Please check this in to the branch today!

Comment 26

[John Keiser (jkeiser)]

fix checked into branch.