iMIP bar listens for event that might have already happened

RESOLVED FIXED in 6.8

Status

defect
RESOLVED FIXED
8 months ago
5 months ago

People

(Reporter: darktrojan, Assigned: darktrojan)

Tracking

(Blocks 1 bug)

unspecified
Dependency tree / graph

Details

()

Attachments

(1 attachment)

Assignee

Description

8 months ago
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

8 months ago
I saw something similar recently but wasn't able to reproduce. Do you have some STR at hand?
Flags: needinfo?(geoff)
Assignee

Comment 2

8 months ago
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)

Comment 3

8 months ago
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.
Assignee

Comment 4

8 months ago
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.

Comment 6

8 months ago
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

Comment 8

7 months ago
Assignee: nobody → geoff
Status: NEW → ASSIGNED
Attachment #9019313 - Flags: review?(philipp)
Assignee

Updated

6 months ago
Attachment #9019313 - Flags: review?(makemyday)

Comment 9

5 months ago
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.
Assignee

Comment 11

5 months ago
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+

Comment 13

5 months ago
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
Last Resolved: 5 months ago
Resolution: --- → FIXED
Assignee

Updated

5 months ago
Target Milestone: --- → 6.8
You need to log in before you can comment on or make changes to this bug.