Compose window is broken with Personas

RESOLVED FIXED in Thunderbird 16.0

Status

Thunderbird
Message Compose Window
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: protz, Assigned: standard8)

Tracking

({regression})

Trunk
Thunderbird 16.0
x86_64
Linux
regression

Thunderbird Tracking Flags

(thunderbird14+ fixed, thunderbird15 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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

5 years ago
I confirm seeing this error too but I didn't see the relation to the compose window. I can check that out.
(Assignee)

Comment 2

5 years ago
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.
tracking-thunderbird14: --- → +
Keywords: regression
(Assignee)

Comment 3

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

Comment 4

5 years ago
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.
(Assignee)

Comment 5

5 years ago
Created attachment 634321 [details] [diff] [review]
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.
Assignee: nobody → mbanner
Status: NEW → ASSIGNED
Attachment #634321 - Flags: review?(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?
Attachment #634321 - Flags: review?(mconley) → review+
(Assignee)

Comment 7

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

See comment 0 and comment 2.
(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.
(Assignee)

Comment 9

5 years ago
Checked in:

https://hg.mozilla.org/comm-central/rev/09ee92ece7f2
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 16.0
(Assignee)

Comment 10

5 years ago
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.
Attachment #634321 - Flags: approval-comm-beta+
Attachment #634321 - Flags: approval-comm-aurora+
(Assignee)

Comment 11

5 years ago
Checked in:

https://hg.mozilla.org/releases/comm-aurora/rev/3d11560ab510
https://hg.mozilla.org/releases/comm-beta/rev/eb5a92da6e74
status-thunderbird14: --- → fixed
status-thunderbird15: --- → fixed
You need to log in before you can comment on or make changes to this bug.