563615 - hideEmailAddressPopup is not defined in the standalone message window

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, defect)

Description

[Mark Banner (:standard8)]

When showing and hiding the menu for a contact, I'm seeing:

Error: hideEmailAddressPopup is not defined
Source File: chrome://messenger/content/messageWindow.xul
Line: 1

This should probably be hideEmailNewsPopup like it is in messenger.xul

Comment 1

[Jim Porter (:squib)]

Attachment: Fix error

This is my fault (fallout from bug 35689), so here's a fix.

Comment 2

[Mark Banner (:standard8)]

Jim: want to request review on this?

Comment 3

[Wayne Mery (:wsmwk)]

Jim, might Bug 565641 also be fallout from bug 35689?

Comment 4

[Jim Porter (:squib)]

Comment on attachment 443389 [details] [diff] [review]
Fix error

Blake, could you take a look at this? It should be simple. It also looks like this is the only place where we missed this: http://mxr.mozilla.org/comm-central/search?string=hideEmailAddressPopup

Comment 5

[Jim Porter (:squib)]

(In reply to comment #3)
> Jim, might Bug 565641 also be fallout from bug 35689?

I think it's actually fallout from bug 550487 (and hence a dupe of bug 563612).

Comment 6

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 443389 [details] [diff] [review]
Fix error

>+++ b/mail/base/content/messageWindow.xul	Tue May 04 12:44:45 2010 -0500
>@@ -121,7 +121,7 @@
>-           onpopuphiding="hideEmailAddressPopup(findEmailNodeFromPopupNode(document.popupNode, 'emailAddressPopup'))">
>+           onpopuphiding="hideEmailNewsPopup(findEmailNodeFromPopupNode(document.popupNode, 'emailAddressPopup'))">

Okay, that makes sense, but I wonder if we should change setupEmailAddressPopup, too.

Either way, r=me.

Later,
Blake.

Comment 7

[Jim Porter (:squib)]

I noticed this hasn't been checked in, but it's passed review. Could someone with commit rights check it in?

Comment 8

[Mark Banner (:standard8)]

Jim: just add checkin-needed to the keywords, and it'll be flagged up.

Note you should probably state if you're going to address comment 6 or not.

Comment 9

[Jim Porter (:squib)]

Alright, thanks. I wasn't sure if I was allowed to do that or not.

As for comment 6, there are separate functions "setupEmailAddressPopup" and "setupNewsgroupPopup", and they'd be tough to merge, but they can both use the same hide function.

Comment 10

[Mark Banner (:standard8)]

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

Comment 11

[Mark Banner (:standard8)]

Comment on attachment 443389 [details] [diff] [review]
Fix error

Will consider for 3.1 as well.

Comment 12

[Mark Banner (:standard8)]

Comment on attachment 443389 [details] [diff] [review]
Fix error

a=Standard8, this seems like the right thing to do here.

Comment 13

[Mark Banner (:standard8)]

Checked into 1.9.2:

http://hg.mozilla.org/releases/comm-1.9.2/rev/8d8b01df69f7