670150 - Installing message pane event handlers before window load event busts many themes

Status: NEW

(Thunderbird :: Theme, defect)

Description

[Eyal Rozenberg]

I'm the author of BiDi Mail UI:
https://addons.mozilla.org/en-US/thunderbird/addon/310/

with Thunderbird 5, two users have reported, that enabling my extension (even on a clean profile, and even with practically all of its functionality disabled) the status bar loses most of its contents:
https://www.mozdev.org/bugs/show_bug.cgi?id=24167

Now, I have no idea why this is happening, as my extension does not touch the status bar at all.

Comment 1

[Ludovic Hirlimann [:Usul]]

Anything in the error console ?

Comment 2

[Eyal Rozenberg]

Nope, nothing. Specifically:
https://www.mozdev.org/bugs/attachment.cgi?id=6967

Comment 3

[Eyal Rozenberg]

A couple of users reported that installing a different theme, specifically, 'Simple Green', makes this problem go away.

Comment 4

[Ludovic Hirlimann [:Usul]]

Andreas can you have a quick look ?

Comment 5

[Mark Banner (:standard8)]

We've seen a few things like this, normally we find that it is due to something touching the DOM before it has completely loaded. In this case I suspect the fact that you're calling installComposeWindowEventHandlers() which does getElementById before the window has fully loaded could be the problem.

Try moving those function calls into an onload handler.

Comment 6

[[e-mail]]

I wrote up an explanation of that problem in a news post somewhere. Basically, touching the DOM in inline script is really bad because it causes frames to be constructed before overlays have finished loading, and the recently added support for Personas simply makes the problem more visible.

Comment 7

[Eyal Rozenberg]

So, I need to do something like:

window.addEventListener('load', BiDiMailUI.MessageOverlay, true);

inline, and have that add the event listeners to the message pane?

Also: This behavior seems somewhat inconsistent. If I manage to get the element by ID and install the handlers, it means things are "ready" for me to do so. If this is not the case, should I not fail? Or, alternatively, should not other code account for the possibility of this having been done?

Comment 8

[[e-mail]]

(In reply to comment #7)
> Also: This behavior seems somewhat inconsistent. If I manage to get the
> element by ID and install the handlers, it means things are "ready" for me
> to do so. If this is not the case, should I not fail? Or, alternatively,
> should not other code account for the possibility of this having been done?
There are two ways we could make XUL overlays work:
1. Lay out the document each time an overlay loads.
2. Lay out the document once when all overlays have loaded.
Option 2 is obviously much faster and avoids strange effects from partly loaded documents, but you have to be careful not to touch the DOM from script. (Partly loaded HTML documents on the other hand are expected and there are other tricks used to speed them up, such as avoiding painting until the first screen loads.)

Comment 9

[Eyal Rozenberg]

Well, this sounds perfectly reasonable, except for the "you have to be careful" part. If I'm not supposed to touch that, don't let me. Make the getElementById fail, or prevent the laying out, or do the layout and become slower due to my negligence - but things can't go bust for doing something that's semantically valid. That's the way I see it anyway.

Comment 10

[Eyal]

I have 3 profiles in my Thunderbird. In two of them "Bidi Mail UI" is installed. In both profiles, disabling the add-on solves the problem. For the workaround involving the "Simple Green" theme to work I have to do the following:
I have to close TB5.0 from the "file" menu (not from the window's title-bar) and avoid the "Thunderbird- Choose User Profile" window by launching the profile I want by:
"C:\Program Files\Mozilla Thunderbird\thunderbird.exe" -p "ProfileName"

OS: WinXP SP3

Comment 11

[:aceman]

So did the change in comment 7 work?

Comment 12

[Wayne Mery (:wsmwk)]

Is this likely still a problem with version 102 or newer?

Comment 13

[John Bieling (:TbSync)]

There was a workaround presented which is straight forward. The old XUL overlay method was able to mess up Thunderbird because it was able to hook in everywhere. Asking core developers to test beyond what the app is supposed to do and think of what add-ons might be doing in the future is a bit too much.

There is still a possibility, that something like this may happen in an Experiment today, but we have an understanding that the Experiment has to make sure it is not breaking Thunderbird, for example by using the presented workaround.

I tested this add-on and I did not see it misbehave in 102.