230347 - Javascript warnings in msgHdrViewOverlay.js (AddExtraAddressProcessing, FinishEmailProcessing, NotifyClearAddresses, gExpandedHeaderView.length, gCollapsedHeaderView[headerName])

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Reducing the headerpane by pressing the '-' box results in following JS warning:

Warning: reference to undefined property this.AddExtraAddressProcessing
Source File: chrome://messenger/content/msgHdrViewOverlay.js
Line: 759

Steps to reproduce:
1. Change header view to another entry
2. Press '-' box again

Comment 1

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.6) Gecko/20040113] (W98SE)

Confirmed from bug 231535 comment 1:
{
The former warning happens the first time I expand the "header pane" of a given
message.
}

The v1.6, and v1.7a-Trunk (v1.111), line 776 is
{
775   
776   if (this.AddExtraAddressProcessing != undefined)
777     AddExtraAddressProcessing(emailAddress, emailAddressNode);
}

(What build is the line 759 from ?)
This is the only occurence of |this.AddExtraAddressProcessing| in the file.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

At the moment I've the latest official nighty build: Mozilla Thunderbird 0.5a
(20040113)

Since my reporting additional warnings are shown. Following I get now:

   1. Warning: reference to undefined property this.AddExtraAddressProcessing
      Source File: chrome://messenger/content/msgHdrViewOverlay.js
      Line: 759

   2. Warning: reference to undefined property gCollapsedHeaderView[headerName]
      Source File: chrome://messenger/content/msgHdrViewOverlay.js
      Line: 561

   3. Warning: reference to undefined property this.FinishEmailProcessing
      Source File: chrome://messenger/content/msgHdrViewOverlay.js
      Line: 595

There are two variations to activate this warnings:

1. "View | Headers | Normal"
By collapsing the header I get all three warnings shown in the order above. An
additional Expand will only raise FinishEmailProcessing.

2. "View | Headers | All"
Collapse the header will raise the same warnings like 1) but additional 12
"gCollapsedHeaderView[headerName]" warnings. A following Expand raises only a
"FinishEmailProcessing" warning.

Comment 3

[Karsten Düsterloh]

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

Comment 4

[Karsten Düsterloh]

Taking. Patch forthcoming.

Comment 5

[Karsten Düsterloh]

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

Comment 6

[Karsten Düsterloh]

Attachment: small fix

I've never seen that warnings before because I already fixed them in Mnenhy.
Oh, my... :)

Comment 7

[[e-mail]]

Comment on attachment 139510 [details] [diff] [review]
small fix

>-      headerEntry = gExpandedHeaderView[headerName];
>-      if (headerEntry == undefined && gViewAllHeaders)
>+      if (headerName in gExpandedHeaderView)
>+        headerEntry = gExpandedHeaderView[headerName];
>+
>+      if (!headerEntry && gViewAllHeaders)
>       {
>         // for view all headers, if we don't have a header field for this value....cheat and create one....then
>         // fill in a headerEntry
>         gExpandedHeaderView[headerName] = new createNewHeaderView(headerName);
>         headerEntry = gExpandedHeaderView[headerName];
>       }
Hmmm... the header sink isn't supposed to send us headers we don't want...

There are two other warnings that you missed, and might like to follow up on.
function CheckNotify()
{
  if (this.NotifyClearAddresses != undefined)
    NotifyClearAddresses();
}
and
gExpandedHeaderView = {};
which causes an error on this line:
if (!gExpandedHeaderView.length)

Comment 8

[Karsten Düsterloh]

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

Comment 9

[Karsten Düsterloh]

Attachment: addressed comments & cleanup

(1)
Addressed the "undefined" problem in function CheckNotify()

(2)
> Hmmm... the header sink isn't supposed to send us headers we don't want...

But it sends everything if we don't know what we want. ;-) 
In the case of "Normal Header View", we predefine a set of headers to show, but
this does not suffice when showing "All Headers". Since we can't know what
headers to expect, we need to create all not precreated.

(3)
gExpandedHeaderView and gCollapsedHeaderView are not "real" arrays, they're
just used as associative arrays / objects and hence don't know about "length".
The function initializeHeaderViewTables testing their .length is only called in
two places that ensure that they are empty, so this buggy test can vanish
completely. (diff shows just some extra whitespace changes there.)

(4)
"isEmailAddress" is never used anywhere, not even in Netscape 7.1's messenger
or aim module. AFAICT, it has never been used since its creation in 1.49...

Comment 10

[David :Bienvenu]

Comment on attachment 139636 [details] [diff] [review]
addressed comments & cleanup

I'm going to switch this sr request to mscott, only because he knows this code
much better than I do.

Comment 11

[Scott MacGregor]

Comment on attachment 139636 [details] [diff] [review]
addressed comments & cleanup

this looks ok to me. I don't remember what we were trying to do with
isEmailAddress:true anymore. 

I looked through the code and I agree it does not look like we use it anywhere
anymore.

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in msgHdrViewOverlay.js;
/cvsroot/mozilla/mailnews/base/resources/content/msgHdrViewOverlay.js,v  <-- 
msgHdrViewOverlay.js
new revision: 1.112; previous revision: 1.111
done

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

verified.