372424 - iTIP/iMIP invitation bar doesn't disappear after changing e-mail folder

Status: RESOLVED FIXED

(Calendar :: E-mail based Scheduling (iTIP/iMIP), defect, P3)

Description

[Roger]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.10) Gecko/20070221 Red Hat/1.5.0.10-0.1.el4 Firefox/1.5.0.10
Build Identifier: Lightning .0.3.1 with wcap enabler 0.3

When clicking on a message with an invitiation, the notice comes up telling you of the fact.  If you click directly on another mail folder, the notice stays in place.  
If you click another email within that mail folder, then the notice goes away like you expect it to.  
When you click on the message *with* the invitation, the 'attachment' icon for the message a two messages up also shows up.  the attachment icon for the second message goes away when the invitation notice disappears.

Reproducible: Always

Steps to Reproduce:
1.click on message with invitaion
2. click on a different folder, notice stays there.
3. click back to a different message in same folder, notice goes away.
Actual Results:  
invitation notice stayed visible after clicking another folder.

Expected Results:  
expected notice to go away after clicking on a different folder.

Comment 2

[Stefan Sitter [:ssitter]]

Marking as confirmed based on duplicate report with Lightning 0.5pre.

Comment 3

[Sebastian Schwieger]

Attachment: using onunload listener of the window

I don't know if this fixes it on Mac and/or Windows. Tested on Linux.

Comment 4

[Przemyslaw Bialik]

Confirming the fix using manually patched Lightning 0.8pre (build 2007112305) on Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.9) Gecko/20071031 Thunderbird/2.0.0.9 ID:2007103104

Comment 5

[Simon Paquet [:sipaq]]

Setting wanted0.8- as the main Calendar developers will not devote any time to
this in the 0.8 timeframe. Patches are, of course, always welcome.

Comment 6

[cmtalbert]

Comment on attachment 289038 [details] [diff] [review]
using onunload listener of the window

>Index: calendar/lightning/content/imip-bar.js
>===================================================================
> function imipOnUnload()
> {
>-    removeEventListener("messagepane-loaded", imipOnLoad, true);
>-    removeEventListener("messagepane-unloaded", imipOnUnload, true);

I'm a little uncertain that removing these is the proper thing to do here.  However, I verified that we lose the ability to display the imip-bar if we keep these lines in.  I'm not sure if the window.addeventhandler uses memory that is managed differently from the way that the simple addeventhandler that we were using before is managed.  I'd like to understand if we are causing a memory leak by not having these in the code.

The patch does indeed work for Linux (taking Sebo's word) and for Windows (I tested that and Przemyslaw confirmed it too).  However, it does NOT work on Mac OS X (Attempted on 10.4).

I'd like to get this patch in since it solves the problem for a 2/3 of our default platforms, but I'd like to know if we are leaking memory by removing the "removeEventListener" commands.  That's the primary issue of importance in my mind.  We also need to figure out if there is some other listener we should use for Mac OS X.  So, in light of these two things, I think I have to minus for now. 

Sebo: Can you answer the question of whether or not this causes a memory leak? 

I'll ask the mac platform folks at MoCo to see if they understand what we need to do here.

Comment 7

[cmtalbert]

Colin, Do you know why this approach would not work on Mac?  Is there something different we need to do here? Is the window.unload listener treated differently on Mac?

Thanks for the help.

Comment 8

[Przemyslaw Bialik]

Sebastian: could you comment on your patch so we can fix this bug for 2 platforms and left bug open as Mac only ?

Comment 9

[Sebastian Schwieger]

Sorry, I don't know if this causes a memory leak.
I did some testing as to why we cannot remove the eventListener in onunload. (In that case the bar is not shown at all.) It looks like the onload and onunload handlers are called multiple times before the message is actually shown. The onunload handler is at least called once before that. This would explain why removing the event listener in onunload has the effect of not showing the bar at all.
The same event listener can only be added once per window. That means there might be a memory leak if it is not removed but we are talking about one listener and not about a whole array of them...

Comment 10

[Sebastian Schwieger]

dmose, could you have a look at this issue, please? Thanks.

Comment 11

[Dan Mosedale (:dmosedale, :dmose)]

Boy, I'd love to switch Thunderbird to using the toolkit notification bar, but I'll be surprised if I get the bandwidth to do this by Tb3.  In any case, I'll have a look.

Comment 12

[David :Bienvenu]

marking p3, since this is relatively rare. It looks bad, but the workaround is easy.

Comment 13

[Ludovic Marcotte]

I've just tried the patch and it does work for me on Mac OS X 10.5.5.

We should nevertheless use:

    hideElement("imip-button1");
    hideElement("imip-button2");
    hideElement("imip-button3");

instead of:

    document.getElementById("imip-button1").setAttribute("hidden", "true");
    document.getElementById("imip-button2").setAttribute("hidden", "true");

Comment 14

[Philipp Kewisch [:Fallen][☁️📆]]

Attachment: Fix - v1

This patch takes care by hooking into the function that hides the header pane. I'd prefer a real event, but I think its the best we can do for now.

Comment 15

[Simon Vaillancourt]

Comment on attachment 432926 [details] [diff] [review]
Fix - v1

r=simon looks good

Comment 16

[Philipp Kewisch [:Fallen][☁️📆]]

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/6ed89063cf94>
-> FIXED

Comment 17

[Philipp Kewisch [:Fallen][☁️📆]]

Backported to comm-1.9.2 <http://hg.mozilla.org/releases/comm-1.9.2/rev/0251686b8374>
a=philipp