579672 - Personas/Lightweight-theme footer not restored after restart (getElementById returns null, incomplete DOM?)

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[gekacheka]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2.6) Gecko/20100625 Firefox/3.6.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2.7) Gecko/20100713 Thunderbird/3.1.1

Selected Persona/Lightweight-theme does not appear in thunderbird 3.1.1 footer.
Same result on
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2.8pre) Gecko/20100716 Lanikai/3.1.2pre
Also happens on fresh profile.


Reproducible: Always

Steps to Reproduce:
1. (optional: start fresh profile)
2. Load Personas Add-on (needed to load lightweight theme)
   Tools/Add-ons/Get Add-ons
   Search "Personas Plus"
   Add to thunderbird/lanikai
   Restart
3. Restart again.
Actual Results:  
After step 2, default theme "Groovy blue" swirl images are shown in both toolbar header and statusbar footer.
After step 3 and later restarts, header has swirl image but footer has no swirl image (only theme background color).
Workaround: Theme footer image can be restored for session by reselecting it (select another theme, then come back).
But after next restart, footer image is missing again.

Expected Results:  
After persona/lightweight-theme is installed, theme images persist in both header and footer.

(footer image depends on fix in Bug 569139, so no footer image in Thunderbird 3.1)

Comment 1

[gekacheka]

Attachment: instrumented modules/LightweightThemeConsumer.jsm

Instrumenting modules/LightweightThemeConsumer.jsm _update shows that 
0.  this._doc.documentElement is the window id="messengerWindow" 
    as it should be 
  http://mxr.mozilla.org/comm-1.9.2/source/mail/base/content/messenger.xul#68
1.  this._doc.getElementById("status-bar") returns null  (!?)
2.  the children of this._doc.documentElement are
    <data id="mail-bar2" ...>
    <stringbundleset id="stringbundleset" ...>
    and many <script> elements, but no <statusbar>  ?

(I don't understand it.  Maybe the DOM is not fully created before this is called?  The status bar clearly exists with the right id by the time I can inspect it with DOM inspector.)

Output:

this._footerId=status-bar
active=true
aData.footerURL=file:///C:/temp/Fresh3.1.1/lightweighttheme-footer?33;
footer=null
this._doc=[object XULDocument]
this._doc.documentElement=[object XULElement]
this._doc.documentElement.tagName=window
this._doc.documentElement.id=messengerWindow
statusbarById=null
statusbarByTag=null
[(nonScript)kidTags#Ids]=data#mail-bar2,stringbundleset#stringbundleset

Comment 2

[Russ Hansen]

I can confirm this problem (using 3.1.1 and fully patched Windows XP SP3).

Comment 3

[Mark Banner (:standard8)]

Attachment: Proposed fix

I think I've found the issue - selectionsummaries is trying to initialise its strings and is accessing the document to do so. This then causes some of the bindings to be loaded - in this case general.xml, which in turn initialises LightweightThemesConsumer which then tries to set the status bar up and fails because we are still in the processing of the js script stage and we haven't fully loaded the chrome of the main window yet.

I've therefore delayed the initialisation of those strings until needed - either in MultiMessageSummary or ThemeSummary constructors. I'm not convinced this is the best place and maybe we should do it in the general mail window startup. Andrew what do you think?

Comment 4

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

Comment on attachment 472362 [details] [diff] [review]
Proposed fix

If this fixes it, let's land it.  Thanks to XBL and its broken creation semantics, some things being script blockers, etc., trying to think about startup deeply is likely to just lead to madness.

Comment 5

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/87ac173b75ac

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 472362 [details] [diff] [review]
Proposed fix

We should also think about this for 3.1.x

Comment 8

[Russ Hansen]

I have applied the fix manually (messenger.jar) to both 3.1.2 and 3.1.3 builds and it resolves the problem with no unwanted start-up problems on my system.

Comment 9

[[e-mail]]

Please don't touch the DOM before it has finished loading. Thank you.

Comment 10

[Russ Hansen]

A small detail I am noticing on 3.1.3 after applying this fix is that the "fox" personas icon (and quick menu) no longer appears in the status bar. The personas option menu still appears in the "tools" menu, however, so no actual functionality is lost.

Comment 11

[Russ Hansen]

Above behavior is consistent with Bug 594732.

Comment 12

[Russ Hansen]

As hinted in Bug 594732, this fix breaks the status bar for Thunderbird 3.1.3. I put the old code back, and then the status bar functions normally, although without lightweight theme applied. If the new code does the same to Thunderbird 3 beta, this bug needs to be reopened.

Comment 13

[Mark Banner (:standard8)]

We only need to re-open this bug if we decide to back out the patch for bug 594732. At the moment, I'd like to take a look at it before then as it doesn't really break anything major. Unfortunately it may be Monday before I get chance to take a look.

Comment 14

[Mark Banner (:standard8)]

Comment on attachment 472362 [details] [diff] [review]
Proposed fix

Not taking for 3.1.5 due to outstanding issues applying on that branch. May take for 3.1.6.

Comment 15

[Mark Banner (:standard8)]

Comment on attachment 472362 [details] [diff] [review]
Proposed fix

Although I was initially hoping to get this into 3.1, the realistic point of view is that there was at least one extension issue that was highlighted with the trunk changes, and there's one other bug still in the 3.1 code that would need fixing as well.

So I think this probably isn't going to be safe enough for 3.1, and we'll just keep it for the next version, which will either be from trunk or possibly 3.2.