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

VERIFIED FIXED

Status

SeaMonkey
MailNews: Message Display
VERIFIED FIXED
14 years ago
13 years ago

People

(Reporter: whimboo, Assigned: Karsten Düsterloh)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

8.77 KB, patch
Scott MacGregor
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

14 years ago
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
[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.
Summary: Javascript Warning: reference to undefined property this.AddExtraAddressProcessing → In <chrome://messenger/content/msgHdrViewOverlay.js>, Javascript "Warning: reference to undefined property this.AddExtraAddressProcessing"
Keywords: testcase
Summary: In <chrome://messenger/content/msgHdrViewOverlay.js>, Javascript "Warning: reference to undefined property this.AddExtraAddressProcessing" → In <chrome://messenger/content/msgHdrViewOverlay.js>, JavaScript "Warning: reference to undefined property this.AddExtraAddressProcessing"
(Reporter)

Comment 2

14 years ago
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.
OS: Windows XP → All
Hardware: PC → All
Summary: In <chrome://messenger/content/msgHdrViewOverlay.js>, JavaScript "Warning: reference to undefined property this.AddExtraAddressProcessing" → Javascript warnings in file msgHdrViewOverlay.js: "Warning: reference to undefined property" for AddExtraAddressProcessing, gCollapsedHeaderView[headerName] and FinishEmailProcessing
(Assignee)

Comment 3

14 years ago
*** Bug 231535 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 4

14 years ago
Taking. Patch forthcoming.
Assignee: sspitzer → mnyromyr
(Assignee)

Comment 5

14 years ago
*** Bug 231551 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 6

14 years ago
Created attachment 139510 [details] [diff] [review]
small fix

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

Updated

14 years ago
Attachment #139510 - Flags: superreview?(bienvenu)
Attachment #139510 - Flags: review?(neil.parkwaycc.co.uk)

Comment 7

14 years ago
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)
Attachment #139510 - Flags: review?(neil.parkwaycc.co.uk) → review+
(Assignee)

Comment 8

14 years ago
*** Bug 231550 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 9

14 years ago
Created attachment 139636 [details] [diff] [review]
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...
(Assignee)

Updated

14 years ago
Attachment #139510 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #139636 - Flags: superreview?(bienvenu)
(Assignee)

Updated

14 years ago
Attachment #139510 - Flags: superreview?(bienvenu)
(Assignee)

Updated

14 years ago
Keywords: testcase
Summary: Javascript warnings in file msgHdrViewOverlay.js: "Warning: reference to undefined property" for AddExtraAddressProcessing, gCollapsedHeaderView[headerName] and FinishEmailProcessing → Javascript warnings in msgHdrViewOverlay.js (AddExtraAddressProcessing, FinishEmailProcessing, NotifyClearAddresses, gExpandedHeaderView.length, gCollapsedHeaderView[headerName])

Comment 10

14 years ago
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.
Attachment #139636 - Flags: superreview?(bienvenu) → superreview?(mscott)

Comment 11

14 years ago
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.
Attachment #139636 - Flags: superreview?(mscott) → superreview+
Checking in msgHdrViewOverlay.js;
/cvsroot/mozilla/mailnews/base/resources/content/msgHdrViewOverlay.js,v  <-- 
msgHdrViewOverlay.js
new revision: 1.112; previous revision: 1.111
done
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
(Reporter)

Comment 13

14 years ago
verified.
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.