498933 - mozmill helper function wait_for_message_display_completion and MessageDisplayWidget.messageLoaded are unreliable

Status: RESOLVED FIXED

(Thunderbird :: Folder and Message Lists, defect)

Description

[Andrew Sutherland [:asuth] (he/him)]

Attachment: v1 wait for the load to occur by watching the docShell and its URI

In mail/test/mozmill/shared-modules/test-folder-display-helpers.js, the function wait_for_message_display_completion claims that it causes us to wait for message load to complete.  It is, at best, unreliable in this endeavor.

The attach patch attempts to resolve the problem by:
- Making the messageLoaded flag (exclusively introduced for mozmill unit testing, but left exposed in the theory that it could be useful to actual UI code) more reliable.  Not perfect, but reliable.  Because we are slaved off of the message pane's sink, we will mark messageLoaded before the URL actually completes running.
- Actually making sure the content pane widget ("messagepane" or whatever is happening) has completed loading.

I wrote this patch before I knew the following, however:

In https://bugzilla.mozilla.org/show_bug.cgi?id=498344#c11, bienvenu said:
> don't know if this helps, but there's a notification you can listen for when a
> message load has completed - nsMsgStatusFeedback will send
> folder->NotifyPropertyFlagChanged("msgLoaded"). 
>
> Why this isn't a folder event I don't know, though I guess the loaded message
> is a property...

This is indeed very useful, and would allow for much more precise maintenance of messageLoaded.  I have a vague concern about ambiguity when there are multiple FolderDisplayWidgets involving the same folder.  Ideally, I think the MessageDisplayWidget would just be a listener to the streaming itself.  Since it is involved/could be more involved in the initiation of the streaming, this would make sense and be resilient to what other FolderDisplayWidget/MessageDisplayWidget instances might be doing.

Since this patch is a solid improvement that addresses the immediate testing problem, I'd propose reviewing the patch on that basis; we then spin off an explicit bug to make MessageDisplayWidget completely aware of the state of its associated message streaming.  (And we can fix-up wait_for_message_display_completion to use that at that point.  Or do something even prettier.)

Comment 1

[David :Bienvenu]

Comment on attachment 383740 [details] [diff] [review]
v1 wait for the load to occur by watching the docShell and its URI

this doesn't fix the assertion I'm seeing with my test case (the patch I made to test-message-window.js for testing stand-alone msg window nav) but it doesn't make it worse either.

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

pushed: http://hg.mozilla.org/comm-central/rev/4864a0fb9857