256049 - Error ``id has no properties'' [xs] in file ``chrome://messenger/content/messengercompose/addressingWidgetOverlay.js'', line 1057, character 0.

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect, P5)

Description

[timeless]

Error ``id has no properties'' [xs] in file
``chrome://messenger/content/messengercompose/addressingWidgetOverlay.js'', line
1057, character 0.
Stopped for error handler.
#0: function awDocumentKeyPress(event=Event:{0}) in
<chrome://messenger/content/messengercompose/addressingWidgetOverlay.js> line 1057
1055: try {
1056: var id = event.target.getAttribute('id');
1057: if (id.substr(0, 11) == 'addressCol1')
1058: awMenulistKeyPress(event, event.target);
1059: } catch (e) { }

Comment 1

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

sorry for the spam.  making bugzilla reflect reality as I'm not working on these bugs.  filter on FOOBARCHEESE to remove these in bulk.

Comment 2

[Jonathan Kamens]

This bug is still present in Thunderbird 3.1.1, although now it's at line 1017 instead of line 1057.

The problematic expression is at line 1016:

1016: var id = event.target.getAttribute('id');

As far as I can tell, in this context, this returns null:

event.target.getAttribute('id')

but this returns a string:

event.target.id

Perhaps fixing this is as simple as changing from "getAttribute('id')" to "id"? I confess to being somewhat confused about the difference between using getAttribute and accessing an object properly, so perhaps I am off-base here.

Although I realize that this may not be a high-priority user-visible issue, it is quite annoying when trying to do JavaScript debugging on the compose window with Venkman, because Venkman insists on stopping because of this error every time a key is pressed in the compose window.

Comment 3

[Jonathan Kamens]

Confirmed: changing from event.target.getAttribute('id') to event.target.id eliminates the JavaScript error while preserving the intended behavior (tab works to move from the first column of the addressee box to the second column).

Also, the reason I saw this error at a different line number is because I was running out of an installed tree and the jar builder removes copyright notices from the js files before packing them into jars.

Trivial fix, how 'bout somebody just commit the thang. ;-)

Comment 4

[Mark Banner (:standard8)]

Jonathan: all changes to code need a patch and a review.

Could you stick one up? I'd suggest ask Neil for review.

Comment 5

[Jonathan Kamens]

Attachment: patch to use property instead of getAttribute

Comment 6

[Jonathan Kamens]

Perhaps I am confused, but I don't seem to have the ability to set who should to the review. And I'm not active enough at this to know what Neil's email address, which I assume I would need to request a review from him, is.

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 462036 [details] [diff] [review]
patch to use property instead of getAttribute

You need to set ?, then you can get reviewer's email addresses from here:

http://www.mozilla.org/about/owners.html

(see under the mailnews section).

Comment 8

[Jonathan Kamens]

How do we get the patch checked in now that it has been reviewed?

Comment 9

[Mark Banner (:standard8)]

Add checkin-needed to the keywords

Comment 10

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/a98ae8d91e45

Comment 11

[Philip Chee]

http://hg.mozilla.org/comm-central/rev/db0b3e35e5b3
Mark, Jonathan do you guys want to backport SeaMonkey's approach to this bug? It involves code simplification and code removal. Win Win!

Comment 12

[Jonathan Kamens]

I don't know the code nearly enough to be able to understand the SeaMonkey fix, and unfortunately I don't really have time to pick it up right now.

It's somewhat odd-seeming to me that the SeaMonkey folks seem to have a much more active development team than Thunderbird.

Comment 13

[Philip Chee]

For whoever picks this up. The rather convoluted code I removed was originally put there to work around ancient bugs that don't exist any more. So we can chuck the workarounds out of the window and simplify the code.