276707 - JS Error when dragging address card with no email address to compose window address widget

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect)

Description

[Mark Banner (:standard8)]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-GB; rv:1.8a6) Gecko/20041212
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a6) Gecko/20050101

When dragging an address card with no email address to the message composition
window, a javascript error is generated and nothing happens.

Reproducible: Always

Steps to Reproduce:
1. Start Mozilla/Address book
2. Create card in address book with no email address.
3. Select compose.
4. Drag created card from address book window to the addressing area of the
message composition window.

Actual Results:  
Nothing visible to the user happened, but the following error was displayed on
the console:

JavaScript error: , line 780:

Expected Results:  
No javascript error and a warning dialog to the user (similar to that recently
implemented in the select addresses dialog, bug 183632).

Comment 1

[Mark Banner (:standard8)]

I think I know what is happening here - found the file via venkman, taking bug,
I will look into it in the next couple of days.

Comment 2

[Mark Banner (:standard8)]

Whilst investigating this bug further, a similar js error appears along with the
following assertions when drag and dropping cards on the Select Address dialog
(from the compose window).

###!!! ASSERTION: bad row index: 'aRowIndex >= 0 && aRowIndex < mRows.Count()',
file nsTreeContentView.cpp, line 356
Break: at file nsTreeContentView.cpp, line 356
###!!! ASSERTION: bad row: 'aRow >= 0 && aRow < mRows.Count()', file
nsTreeContentView.cpp, line 346
Break: at file nsTreeContentView.cpp, line 346

Note: The assertions appear for any card that is dragged and dropped.

Comment 3

[Mark Banner (:standard8)]

Attachment: Fix JS errors

This patch fixes the js error mentioned above and brings up a dialog notifying
the user that they cannot add a card with an empty email address.

I have found no cause for the assertions in the mailnews code, and it only
happens when an address is dragged onto an empty tree row (contrary to what I
said in my previous comment) but doesn't seem to affect functionality
otherwise. Therefore I intend to raise a seperate bug on layout (?) to request
the fix of the assertions.

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 171408 [details] [diff] [review]
Fix JS errors

Requesting sr.

Please note I have now raised bug 278630 about the debug assertions.

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 171408 [details] [diff] [review]
Fix JS errors

Based on Scott's comments about DnD and not producing dialogs as a result of a
DnD, cancelling request for sr until I can revise this patch.

Comment 6

[Mark Banner (:standard8)]

Attachment: Revised patch - removed dialogs.

This patch has been revised, such that if a card with an empty email address is
dragged to an addressing widget (or on the select addresses dialog), can drop
is set false, the drag is denied and no assertions are given.

I've also provided a similar fix for the thunderbird compose dialog.

Comment 7

[David :Bienvenu]

Comment on attachment 175723 [details] [diff] [review]
Revised patch - removed dialogs.

seems OK - however, is there a common place we can put this duplicated code,
shared by the address book and the compose window?

Comment 8

[Mark Banner (:standard8)]

Attachment: Revised incorporate code into common place

I've looked at this again, and the most appropriate place to put the common
code seems to be abDragDrop.js, as it happens, thunderbird hasn't branched this
yet, so can also share the benefits of the same file.

If you don't like the look of it this way, then I'll use the previous patch if
that's ok.

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 175723 [details] [diff] [review]
Revised patch - removed dialogs.

Obsoleting "old" patch.

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 176180 [details] [diff] [review]
Revised incorporate code into common place

Dmose seems to be a bit busy at the moment - requesting sr from scott instead.

Comment 11

[Mark Banner (:standard8)]

Scott, any chance of an sr on this one, although it's not major I'd like to get 
it in soon. Thanks.

Comment 12

[Mark Banner (:standard8)]

Comment on attachment 176180 [details] [diff] [review]
Revised incorporate code into common place

Requesting approval for checking, this is a tidy up patch for drag and drop
issues with address book items for both suite & thunderbird. Should be low
risk.

Comment 13

[chris hofmann]

Comment on attachment 176180 [details] [diff] [review]
Revised incorporate code into common place

a=chofmann

Comment 14

[Mark Banner (:standard8)]

Patch checked in by pike:
2005-05-02 09:08	axel%pike.org 	mozilla/ mailnews/ compose/ resources/ content/
addressingWidgetOverlay.xul 	1.66 	3/2
2005-05-02 09:08	axel%pike.org 	mozilla/ mailnews/ compose/ resources/ content/
addressingWidgetOverlay.js 	1.89 	1/41 
2005-05-02 09:08	axel%pike.org 	mozilla/ mailnews/ addrbook/ resources/ content/
abSelectAddressesDialog.js 	1.41 	2/39 
2005-05-02 09:08	axel%pike.org 	mozilla/ mailnews/ addrbook/ resources/ content/
abDragDrop.js 	1.21 	74/0 
2005-05-02 09:08	axel%pike.org 	mozilla/ mailnews/ addrbook/ resources/ content/
abSelectAddressesDialog.xul 	1.47 	3/2 

Marking as fixed (note there is a seperate bug about the debug assertions - bug
278630)

Comment 15

[Stephen Donner [:stephend] Not actively reading bugmail]

In build 2005-07-10-05 on Windows XP SeaMonkey trunk, dragging an entry with no
email address from the Address Book results in the 'no drop target' icon correctly.

Tested also to ensure that we still correctly follow through if the entry DOES
have an email address.

Verified FIXED.