bugzilla.mozilla.org will be intermittently unavailable on Saturday, March 24th, from 16:00 until 20:00 UTC.

When invoked through a html form, the compose window appears only once




Layout: Form Controls
16 years ago
16 years ago


(Reporter: daisuke, Assigned: John Keiser (jkeiser))


Windows 2000

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [FIX], URL)


(1 attachment, 1 obsolete attachment)

3.93 KB, patch
John Keiser (jkeiser)
: review+
John Keiser (jkeiser)
: superreview+
: approval+
Details | Diff | Splinter Review


16 years ago
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.9+)
BuildID:    20020410

When there is a form that starts up the mail composer in an
html page, the composer only successfully comes up on the first invokation. From
the second invokation on, the clicks to the button are swiftly ignored.

However, if you reload the page, the same button works again, but only once. I
think this has been happening for a while.
( last couple of weeks? )

Reproducible: Always
Steps to Reproduce:
1. go to any page which contains a form like so:

<form action="mailto:foo@bar.com">
  <input type="submit" value="mail me!">

2. press the button. either send the mail, or just close the compose window
3. press the button again. No window comes up
jkeiser, this sounds like a possible result of your "prevent duplicate
submissions" fix...

Comment 2

16 years ago
It's mine, but I don't think I'll deliberately fix it unless it's part of bug
61893.  There's very little point otherwise.  Still unconfirmed, and may even be
working in recent builds, because I haven't had time to turn the HTML below into
a testcase.
Assignee: rods → jkeiser

Comment 3

16 years ago
While I don't consider this to be a real high priority thing,
I really don't think this conforms to what people would expect.

Also, there is some cultural reason here... a lot of people in the
Japanese online community has what's known as an "empty-mail" button
on their webpages. This is used as an explicit sign that you have
visited the site. It's also used as a way to communicate that you
approve of certain things on their pages, in which case you hit the
"empty-mail" button multiple times. I don't think this phenomenon
exist in the U.S., but it's surely there.

Besides, all other major browsers allow that to happen. People are
already used to it. And bringing up the mail composer multiple times
certainly doesn't do any major harm. So why not?

I know it sounds stupid, but I'm pretty sure that other Japanese
surfers like myself *will* find this a nuisance...

Comment 4

16 years ago
(1) the mail compose window *does* open.
(2) if a site wants an empty mail window it can do a mailto: link rather than
form post.
(3) I'm not saying it's not a bug.  I'm just saying I'm not spending the time to
fix it until I fix mailto form submit.

That said, please try a recent nightly.  I know some work has gone into the tree
recently that may have already fixed this.

Comment 5

16 years ago
Taking your suggestion, I tried the 20020416 build. same deal :(

For your point (1),I'm not sure what you meant by "the mail compose window *does* 
open", because from the seond invokation on, it definitely doesn't come up for 
me. Do you actually get the compose window from the second invokation on? Am I
missing something?

For point (2), while appreciated, I think a workaround is besides the point. I 
would think the problem is the fact that something that used to work doesn't any 

And finally for Point (3), sorry, I guess I misread your orignal comment.
I thought it was a flat out no. Understood...

Comment 6

16 years ago
(1) was the fact that it opens once.  If it didn't open at all it would be worse


16 years ago
QA Contact: madhur → tpreston
Severity: normal → minor
Ever confirmed: true


16 years ago
Depends on: 61893

Comment 7

16 years ago
OK, this is still happening, and I've diagnosed the problem.  Mail compose from
LoadURI is returning a channel, but it is not sending the START and STOP events,
so since we never receive a STOP event.  Based on conversations I had with Rick
a couple of months ago, I believe this violates the rules of the interface and
should be fixed in mail/news.  Should it not be returning a channel at all?  Or
is it just that the channel needs to send dummy stop/start requests?

Rick, Darin, Jean-Francois, which of these sounds like the right thing?  Or am I
off base?

Comment 8

16 years ago
hey john,

it appears that the 'mailto' channel is not adding itself to a loadgroup when it
is opened...  take a look at:

In order for the nsIWebProgressListener to fire notifications, each channel must
be added/removed from the correct loadgroup...  since this channel doesn't
handle loadgroups (at all), no notifications get fired.

The fix is to have the mailto channel add/removed itself from the correct
loadgroup (just like it fires OnStart/OnStop events) ...

-- rick
I cannot really anwser that question as this chanel thing append way before
message compose get invoked! cc'ing mscott who I hope should be able to answer it

Comment 10

16 years ago
Created attachment 93265 [details] [diff] [review]

Both form submit and mailto had troubles.  I updated the mailto channel to add
/ remove itself from the load group and to report that it is *not* pending
(pending is defined as "STOP has not fired yet").  And I updated form submit to
check whether a request is pending before listening to web progress.  All is
well with mailto now, and double form submit protection still works for http.


16 years ago
Whiteboard: [FIX]

Comment 11

16 years ago
Comment on attachment 93265 [details] [diff] [review]

>Index: mailnews/compose/src/nsSmtpService.cpp

>+  mLoadGroup = aLoadGroup;
> 	return NS_OK;
> }

indentation fu!  just make it consistent with the
existing file (whatever that means)

> NS_IMETHODIMP nsMailtoChannel::AsyncOpen(nsIStreamListener *listener, nsISupports *ctxt)
> {
>+  // Add to the load group to fire start event
>+  if (mLoadGroup) {
>+    mLoadGroup->AddRequest(this, ctxt);
>+  }
>   mStatus = listener->OnStartRequest(this, ctxt);
>   // If OnStartRequest(...) failed, then propagate the error code...
>@@ -449,6 +457,11 @@
>   // Call OnStopRequest(...) for correct-ness.
>   (void) listener->OnStopRequest(this, ctxt, mStatus);
>+  // Remove from the load group to fire stop event
>+  if (mLoadGroup) {
>+    mLoadGroup->RemoveRequest(this, ctxt, NS_OK);
>+  }

yuck... OnStartRequest, OnStopRequest are not supposed to
be called recursively from AsyncOpen.  that is trouble
waiting to happen.  maybe we're okay because of the way
this channel is used, but watch out!

Attachment #93265 - Flags: review+

Comment 12

16 years ago
In this call to RemoveRequest(...) i think that 'mStatus' should be passed
instead of NS_OK so listeners recieve the same status as that passed to

+  // Remove from the load group to fire stop event
+  if (mLoadGroup) {
+    mLoadGroup->RemoveRequest(this, ctxt, NS_OK);
+  }

Other than that... it looks good.

-- rick

Comment 13

16 years ago
Comment on attachment 93265 [details] [diff] [review]

sr=rpotts@netscape.com (with change to RemoveRequest)
Attachment #93265 - Flags: superreview+

Comment 14

16 years ago
Created attachment 93472 [details] [diff] [review]
Patch v1.0.1

Fix bits from darin and rpotts
Attachment #93265 - Attachment is obsolete: true

Comment 15

16 years ago
Comment on attachment 93472 [details] [diff] [review]
Patch v1.0.1

Carrying r=darin, sr=rpotts
Attachment #93472 - Flags: superreview+
Attachment #93472 - Flags: review+
Comment on attachment 93472 [details] [diff] [review]
Patch v1.0.1

Approved for trunk
Attachment #93472 - Flags: approval+

Comment 17

16 years ago
Fix checked in.
Last Resolved: 16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.