190153 - reply, click in addressing widget, copies first address

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect)

Description

[(not reading, please use [e-mail] instead)]

reply, click in addressing widget, copies first address

let me screen shot

Comment 1

[(not reading, please use [e-mail] instead)]

Attachment: screen shot

this is a recent regression

Comment 2

[(not reading, please use [e-mail] instead)]

cc neil, in case he has cycles to help

Comment 3

[(not reading, please use [e-mail] instead)]

to reproduce:

hit reply
click on empty addressee area
we copy the first one
will happen again on another empty addressee area.

doesn't seem to happen on new message.

Comment 4

[esther]

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

Comment 5

[esther]

Bug 190099 is a dup and has a different scenario to test.

Comment 6

[[e-mail]]

Attachment: Proposed patch

Comment 7

[Jean-Francois Ducarroz]

This is a sensitive area which need carefull testing. We have seen in the past
problem when pre-filling a text field using .value instead of set Attribute. The
problem was that the data retreive when the message is sent was the original
pre-filled value and not what the user had typed.

Try the following:

1) Reply to a message, change the "to:" address and sent the message. Verify it
went to the expected recipient.

2) Reply all to a message address to several recipients (about 5), scroll up and
down the addressing widget and check we don't lose any recipient. Also sent the
message and verify the recipient...

Comment 8

[(not reading, please use [e-mail] instead)]

neil, any idea what caused the regression?

could it be related to the recent checkin to autocomplete.xml?

mozilla/xpfe/components/autocomplete/resources/content/autocomplete.xml

band-aid for bug 90337 URL bar not responsive to the "Enter/Return" key. r=ere
sr=jag

Comment 9

[(not reading, please use [e-mail] instead)]

backing out the bandaid for fix bug #90337 makes this problem go away.

re-assign to neil.

neil, does that change how you want to fix this?

Comment 10

[[e-mail]]

I was thinking about this when ducarroz mentioned that setAttribute("value") was
a workaround. The problem is that if you set .value "too early" then it hides
the current value. Ideally this would never happen, so the band-aid from bug
90337 works around it by unhiding the current value. I'm not clear yet as to why
the two workarounds conflict but if the cause of bug 90337 is fixed probperly
then you won't have to workaround it anyway :-)

Comment 11

[[e-mail]]

Attachment: Duplicate hack

Well, seeing as you don't want to remove your workaround,
I've had to duplicate it instead.

Comment 12

[(not reading, please use [e-mail] instead)]

can you include a comment (like you did for the other checkin) about the band aid?

// XXX bug 90337 band-aid until we figure out what's going on here

also, the mailing list dialog has this problem too.

I'll try out our patch, and if it fixes it, land it.

Comment 13

[(not reading, please use [e-mail] instead)]

> Well, seeing as you don't want to remove your workaround

I don't think ducarroz is against removing the work around, he's concerned about
regressions.

Comment 14

[(not reading, please use [e-mail] instead)]

neil's hack works for the compose window, but there's a similar bug in the 
mailing list dialog, the "fix" is the same.

I'll attach a patch that has both fixes.

Comment 15

[(not reading, please use [e-mail] instead)]

Attachment: hack (neil's hack, in one more place, with comments)

Comment 16

[(not reading, please use [e-mail] instead)]

neil, since your propose patch is not a hack, let's spin that off to a new bug
(http://bugzilla.mozilla.org/attachment.cgi?id=112411&action=view) so we can 
bake it and full test for regressions.

(given our resources, and since we are trying to get to 1.3 beta, let's go low 
risk for now)

Comment 17

[(not reading, please use [e-mail] instead)]

fix checked in, a=blizzard, r/sr=sspitzer

neil, can you spin up a new bug about your first patch (and/or backing out 
these band-aids?)

Comment 18

[Jean-Francois Ducarroz]

correct/ Would be nice if we can remove the work around. But this area of the
code already broke couple times in the past 4 years causing very bad regression.
All I am asking is to do intensive test to be sure what the user see/enter is
what will be really sent...

Comment 19

[(not reading, please use [e-mail] instead)]

Comment on attachment 112464 [details] [diff] [review]
Duplicate hack

clearing out review requests.

this patch should be moved to a new bug

Comment 20

[(not reading, please use [e-mail] instead)]

Comment on attachment 112411 [details] [diff] [review]
Proposed patch

clearing review requests.

Comment 21

[esther]

Using trunk builds 20030211 linux, 20030214 winxp and 200306 maco ox this is
fixed. Verified.