758747 - Compose window is broken with Personas

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Jonathan Protzenko [:protz]]

Timestamp: 05/25/2012 09:43:33 PM
Error: TypeError: footer is null
Source File: resource:///modules/LightweightThemeConsumer.jsm
Line: 60

This may be related to the fact that I'm unable to use the Insert and Format menu entries.

When we initially checked in support for personas in the composition window, I remember struggling with a similar error. The reason was accessing a XBL binding at script load-time (not page load-time). It would trigger evaluation of all XBL bindings, thus triggering the Persona code before the page DOM was complete.

The XBL binding was simply gComposeBundle, which was queried at script load-time. Maybe someone reintroduced this by accident?

Comment 1

[:aceman]

I confirm seeing this error too but I didn't see the relation to the compose window. I can check that out.

Comment 2

[Mark Banner (:standard8)]

You can debug this sort of issue by putting the statement "debugger;" in around line 60 of that LightweightThemeConsumer.jsm, and then you'll get something like:

------------------------------------------------------------------------
Hit JavaScript "debugger" keyword. JS call stack...
0 anonymous([object Object]) ["resource://gre/modules/LightweightThemeConsumer.jsm":61]
    this = [object Object]
1 LightweightThemeConsumer() ["resource://gre/modules/LightweightThemeConsumer.jsm":17]
    this = [object Object]
2 <TOP LEVEL> ["<unknown>":0]
    <failed to get 'this' value>
3 () ["chrome://global/content/bindings/general.xml":74]
    this = [object XULElement @ 0x12fd58660 (native @ 0x12fe74af0)]
4 <TOP LEVEL> ["chrome://messenger/content/messengercompose/cloudAttachmentLinkManager.js":500]
    this = [object ChromeWindow @ 0x128aa3be0 (native @ 0x11e925480)]
------------------------------------------------------------------------

and bingo!

There's actually two places that are at issue for us:

http://hg.mozilla.org/comm-central/annotate/27498a828ee9/mail/components/compose/content/cloudAttachmentLinkManager.js#l499
http://hg.mozilla.org/comm-central/annotate/27498a828ee9/mail/components/compose/content/bigFileObserver.js#l289

commenting those out "fix" the compose window with personas.

What's happening is the document.documentElement is trying to touch the DOM too early, and causing things to be initialised before they are ready.

I've not thought of a way around this yet without altering the compose code - the problem is that we really want to set up these event listeners in the window onload event but that may or may not happen before the ComposeLoad gets called from the window's onload, and the functions called from ComposeLoad are the ones that trigger the compose-window-init event.

Comment 3

[Mark Banner (:standard8)]

David/Mike, is there some other way of hooking up this event that we could use?

Comment 4

[David :Bienvenu]

I would think there is an easier way, but perhaps we could use compose listener events like extensions do. You register a compose listener event upon getting a compose-window-init event, and then listen for ComposeFieldsReady or ComposeBodyReady. At that point, you're pretty much guaranteed that the dom is ready, though maybe it's too late in the process; Or we could add a new compose-window- event, for when the dom is complete.

Comment 5

[Mark Banner (:standard8)]

Attachment: The fix

Originally I thought that changing to window.addEventListener was the way to fix this, but I hadn't thought about changing the final parameter, changing that to true makes the window version work, fixes the lightweight theme issue and we still pass the cloudfile mozmill tests.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 634321 [details] [diff] [review]
The fix

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

Looks good, and fixes the issue. Cloudfile tests como

Do we know *why* this fixes the issue?

Comment 7

[Mark Banner (:standard8)]

(In reply to Mike Conley (:mconley) from comment #6)
> Do we know *why* this fixes the issue?

See comment 0 and comment 2.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

(In reply to Mark Banner (:standard8) from comment #7)
> (In reply to Mike Conley (:mconley) from comment #6)
> > Do we know *why* this fixes the issue?
> 
> See comment 0 and comment 2.

Right - but I'm unsure why the useCapture argument being set to true solves the problem. *sigh*, time to read http://www.w3.org/TR/DOM-Level-3-Events/#event-flow I guess.

Comment 9

[Mark Banner (:standard8)]

Checked in:

https://hg.mozilla.org/comm-central/rev/09ee92ece7f2

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 634321 [details] [diff] [review]
The fix

[Triage Comment]
I'm going to accelerate this onto aurora/beta so that we can get it fixed for the next release.

Comment 11

[Mark Banner (:standard8)]

Checked in:

https://hg.mozilla.org/releases/comm-aurora/rev/3d11560ab510
https://hg.mozilla.org/releases/comm-beta/rev/eb5a92da6e74