Closed Bug 464448 Opened 16 years ago Closed 15 years ago

unable to add more recipients after changing accont from drop down menu

Categories

(Thunderbird :: Message Compose Window, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b2

People

(Reporter: gforg, Assigned: sorin.savu)

References

Details

Attachments

(2 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; hu; rv:1.9.0.3) Gecko/2008092417 Firefox/3.0.3
Build Identifier: 2.0.0.17 (20080914)

Using latest build, disabled all the add-ons.

Have 3 pop3 accounts set up.

lets chose the first one. Click on new mail. The default sender identity is set according to the chosen first account.

Open the drop down list, and choose the second sender identity.

Then enter a recipient, press enter, and the entered text will be selected, not to enter a second recipient. Even manually could there be anything else added in the second line, nor from the adress book, right-clicking a recipient.

Close the compose mail window. Click New mail again. Lets see the drop down list: it shows 6 sender identities, every ident. is shown twice.

Restart TB...

Annoying a bit. Thanks.


Reproducible: Always

Steps to Reproduce:
1. As written at the details...
2.
3.
Actual Results:  
Nothing serious... I got older 2 mins.

Expected Results:  
A freshy composed mail, with many recipients.
This happens also in Linux. 
For me it's annoying because I can't forward mail received to one account  using another account. 
version 2.0.0.19 (20081209)
First this is my first patch ever, so, if I got something wrong, please point it out, I have worked on the MOZILLA_1_8_BRANCH, which from what I understud is the branch for Thunderbird 2.0.0.20pre. 

How to reproduce the bug:
The current bug description is not entirely accurate, this bug only reproduces if there is a ReplyTo ar Bcc: set on the identity where you start, because that is when the function awCleanupRows is called.

What is happening:
When the function awCleanupRows deletes a row, it does not reset the ids for the     menulist and textbox (and from what the code does, it should not do that), but it assumes that the correspondence between the row and the ID is the still ok, by sending to the awRemoveRow the row derived from the element not the real list row. This in terms deletes the wrong row, and in the next iteration, inputElem is null. Here something strange happens, instead of trowing an error (in line 430), something like "inputElem has no property value" .. it silently stops/crashes. This leaves the addressing widget in an "odd" state and results in things like:
  can't add new recipients,
  a new recycled window will have the identities replicated a few times, old attachment entries.
This the root cause of what happens, but I haven't been able to fix it, and this bug is not probably the the best place to do it. Someone with more experience should probably clarify this issue, and submit a proper bug report, more related to the code, I don't feel like I have sufficient knowledge about the mozilla js engine, to do that.  


What gets fixed:
I have changed the way the row gets deleted, by using a function already available awGetRowByInputElement, that finds the real row where the element is.
Attachment #356315 - Flags: superreview?
Attachment #356315 - Flags: review?(bugzilla)
Attachment #356315 - Flags: superreview?(neil)
Attachment #356315 - Flags: superreview?
Attachment #356315 - Flags: review?(bugzilla)
Attachment #356315 - Flags: review?(bienvenu)
Comment on attachment 356315 [details] [diff] [review]
get the correct row to delete when cleaning the addressing widget.

Note: Thunderbird 2 is a stable branch. New development for Thunderbird 3 is being carried out in the comm-central Mercurial repository, although that does not mean that a small patch like this won't be accepted for the next point release.

As a member of the SeaMonkey council, I would also appreciate it if this patch could be ported to the mailnews/ version of this file, this should be straightforward as I don't think we have any significant changes in that area.
Attachment #356315 - Flags: superreview?(neil) → superreview+
I've updated the patch to apply the changes also to mailnews/compose/resources/content/addressingWidgetOverlay.js
Attachment #356315 - Attachment is obsolete: true
Attachment #356327 - Flags: review?(bienvenu)
Attachment #356315 - Flags: review?(bienvenu)
I can't reproduce any of this on my nightly trunk build. I'll try a 2.0x build...
(In reply to comment #5)
> I can't reproduce any of this on my nightly trunk build. I'll try a 2.0x
> build...

It should reproduce there also, I've checkout the code from comm-central Mercurial, and the same issue is still present in the code, I haven't compiled the thing yet.As noted above, it only reproduces when you have a ReplyTo ar Bcc: set on the identity where you start ( I mean in the identity settings under Reply-to Address and/or Identity Setting => Copies & Folders => BCC this address)
I happen to already have a reply-to address for a few of my identities, so that's not the issue. Does the second identity you choose have a reply-to address?
The second identity has no Reply-to but that should have any importance.
(In reply to comment #8)
> The second identity has no Reply-to but that should have any importance.

Actually, I just realized that it must have both Reply-to and BCC to happen.
(In reply to comment #9)
> (In reply to comment #8)
> > The second identity has no Reply-to but that should have any importance.
> 
> Actually, I just realized that it must have both Reply-to and BCC to happen.

I mean, the first identity should have both Reply-to and BCC for the bug to manifest itself.
This patch applies to the latest Thunderbird tree, but it is not tested, as I haven't yet got the time to compile the TB 3 tree.
Attachment #356356 - Flags: superreview?(neil)
Attachment #356356 - Flags: review?(bienvenu)
Attachment #356356 - Attachment is patch: true
Attachment #356356 - Flags: superreview?(neil) → superreview+
Actually I think I managed to reproduce the bug with only reply-to and it took me a few tries but the symptoms are exactly as described here.
(In reply to comment #12)
> Actually I think I managed to reproduce the bug with only reply-to and it took
> me a few tries but the symptoms are exactly as described here.

Probably, you have to try a few times, so the correspondence between the list row and the imputElem id to be lost.

For a detailed step by step on how to reproduce this see bug 163701. After creating the patch, I found bug 163701 and bug 395391 which are the duplicates of this one (actually, this is a duplicate of 163701 but the patch is here), can someone mark them as duplicates of this one ?
(In reply to comment #2)
> Here something strange happens, instead of
> trowing an error (in line 430), something like "inputElem has no property
> value" .. it silently stops/crashes. 
Never mind this, the error is there, I missed it somehow. 

> This the root cause of what happens, but I haven't been able to fix it, and
> this bug is not probably the the best place to do it. Someone with more
> experience should probably clarify this issue, and submit a proper bug report,
> more related to the code, I don't feel like I have sufficient knowledge about
> the mozilla js engine, to do that.  
Clearly, that's not the case.
Assignee: nobody → sorin.savu
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Windows XP → All
Hardware: x86 → All
Comment on attachment 356356 [details] [diff] [review]
The patch above for the tb 3 tree.

ok, I was finally able to reproduce this and verify that this fixes it. Thx for the patch, I've landed it on the trunk.
Attachment #356356 - Flags: review?(bienvenu) → review+
marking fixed on trunk.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment on attachment 356327 [details] [diff] [review]
updated patch per niel's request to contain also the mailnews/ version of the file

I don't know if we'd take this on the 1.8.1 branch since it's not a critical bug or regression, but r+ anyway.
Attachment #356327 - Flags: review?(bienvenu)
Attachment #356327 - Flags: review+
Attachment #356327 - Flags: approval1.8.1.next?
(In reply to comment #19)
> (From update of attachment 356327 [details] [diff] [review])
> I don't know if we'd take this on the 1.8.1 branch since it's not a critical
> bug or regression, but r+ anyway.

It's a simple patch, just one line modified, I've been waiting for it to be included in 2.0.0.x for long time, I can't upgrade yet to 3 ... it has my vote.

Thank you for the review.
Target Milestone: --- → Thunderbird 3.0b2
Comment on attachment 356327 [details] [diff] [review]
updated patch per niel's request to contain also the mailnews/ version of the file

We have too few resources to introduce risk onto the branch for anything other than real security and stability issues; this doesn't fit into either of those buckets.  Apologies.
Attachment #356327 - Flags: approval1.8.1.next? → approval1.8.1.next-
You need to log in before you can comment on or make changes to this bug.