1493034 - iMIP bar listens for event that might have already happened

Status: RESOLVED FIXED

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

Description

[Geoff Lankow (:darktrojan)]

The "messagepane-loaded" event happens during window loading, which could be before the listener is added. There should be a check to see if it has already happened.

https://hg.mozilla.org/comm-central/file/tip/calendar/lightning/content/imip-bar.js#l511

Comment 1

[[:MakeMyDay]]

I saw something similar recently but wasn't able to reproduce. Do you have some STR at hand?

Comment 2

[Geoff Lankow (:darktrojan)]

No, someone suggested it in another bug, and on inspection they appear to be right. I guess reliable STR would be disable Lightning, restart, enable Lightning.

Comment 3

[[:MakeMyDay]]

Hmm, to that regard, that would be a fallout of the wx conversion since before enable Lightning would have required a restart. I don't think that is what I saw.

Comment 4

[Geoff Lankow (:darktrojan)]

Yes, it would be. At some point I knew I should mark this bug as such, but then didn't.

Comment 5

[Magnus Melin [:mkmelin]]

I wonder if this is why for some messages the bar doesn't show in the 3pane, but then show if opened in the stand-alone message window.

Comment 6

[[:MakeMyDay]]

I haven't seen such a bevaviour of a missing imipbar yet - can you describe the circumstances you are observing that? And if that is already an issue for 52 or 60?

Since there isn't a message selected by default in the 3-pane view when starting TB, there shouldn't be a race condition at (application) load time.

Comment 7

[Magnus Melin [:mkmelin]]

I only use trunk :p
Only seen it a few times, during the last few weeks and never before.

Comment 8

[Geoff Lankow (:darktrojan)]

Attachment: 1493034-imip-bar-1.diff

Comment 9

[[:MakeMyDay]]

Comment on attachment 9019313 [details] [diff] [review]
1493034-imip-bar-1.diff

Review of attachment 9019313 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, the patch needs a rebase. I'm fine fine the calendar change, maybe the underscrore should be removed from the _loaded property name. But I'll hand that over to a mail peer since you need such a review anyway when changing also mail code.

Comment 10

[Magnus Melin [:mkmelin]]

Comment on attachment 9019313 [details] [diff] [review]
1493034-imip-bar-1.diff

Review of attachment 9019313 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/base/content/msgHdrView.js
@@ +255,5 @@
>  
>    // Dispatch an event letting any listeners know that we have loaded
>    // the message pane.
>    var headerViewElement = document.getElementById("msgHeaderView");
> +  headerViewElement._loaded = true;

Appears to me you just keep setting this to true on the same msgHeaderView element? And never unsetting it.

Comment 11

[Geoff Lankow (:darktrojan)]

But don't we only ever unload the pane when the window is closing?

Comment 12

[Magnus Melin [:mkmelin]]

Comment on attachment 9019313 [details] [diff] [review]
1493034-imip-bar-1.diff

Review of attachment 9019313 [details] [diff] [review]:
-----------------------------------------------------------------

Yes you're right. r=mkmelin
I'd remove the underscore though

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e1b29b3607c4
Check if header view has already loaded at iMIP bar startup; r=MakeMyDay,mkmelin DONTBUILD