iMIP bar listens for event that might have already happened

RESOLVED FIXED in 6.8

Status

defect
RESOLVED FIXED
10 months ago
7 months ago

People

(Reporter: darktrojan, Assigned: darktrojan)

Tracking

(Blocks 1 bug)

unspecified
Dependency tree / graph

Details

()

Attachments

(1 attachment)

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
I saw something similar recently but wasn't able to reproduce. Do you have some STR at hand?
Flags: needinfo?(geoff)
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.
Flags: needinfo?(geoff)
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.
Yes, it would be. At some point I knew I should mark this bug as such, but then didn't.
Blocks: 1478608
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.
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.
I only use trunk :p
Only seen it a few times, during the last few weeks and never before.
Assignee: nobody → geoff
Status: NEW → ASSIGNED
Attachment #9019313 - Flags: review?(philipp)
Attachment #9019313 - Flags: review?(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.
Attachment #9019313 - Flags: review?(philipp)
Attachment #9019313 - Flags: review?(mkmelin+mozilla)
Attachment #9019313 - Flags: review?(makemyday)
Attachment #9019313 - Flags: review+
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.
But don't we only ever unload the pane when the window is closing?
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
Attachment #9019313 - Flags: review?(mkmelin+mozilla) → review+
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/e1b29b3607c4
Check if header view has already loaded at iMIP bar startup; r=MakeMyDay,mkmelin DONTBUILD
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → 6.8
You need to log in before you can comment on or make changes to this bug.