500506 - Simplify providing an nsIController implementation for tab modes

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, defect)

Description

[Karsten Düsterloh]

Attachment: Switch order of parameters of nsIController implementation and simplify yonder usage

Any tab mode can provide an nsIController implementation to (de)activate commands. But the order of parameters is different from that expected by nsIController, hence we can't just inherit from other nsIController implementations.

Comment 1

[Mark Banner (:standard8)]

Attachment: un-bitrotted version

I think I unbitrotted this right, but I'm seeing this js error on startup:

JavaScript error: chrome://messenger/content/mailWindowOverlay.js, line 2166: DefaultController is not defined

though the code seems to actually be working fine. So I'm not quite sure what is going on here.

(note, no sr needed for mail/).

Comment 2

[Karsten Düsterloh]

Attachment: only use existing default controllers

> JavaScript error: chrome://messenger/content/mailWindowOverlay.js, line 2166:
> DefaultController is not defined
> 
> though the code seems to actually be working fine. So I'm not quite sure what
> is going on here.

That's because TB puts the tabmail object into mailWindowOverlay.js, which gets drawn into SearchDialog.xul.
I also forgot the standalone message window in the first version.
This patch reflects the current state of the SMTabMail patch.

Comment 3

[Mark Banner (:standard8)]

(In reply to comment #2)
> Created an attachment (id=396495) [details]
> only use existing default controllers
> 
> > JavaScript error: chrome://messenger/content/mailWindowOverlay.js, line 2166:
> > DefaultController is not defined
> > 
> > though the code seems to actually be working fine. So I'm not quite sure what
> > is going on here.
> 
> That's because TB puts the tabmail object into mailWindowOverlay.js, which gets
> drawn into SearchDialog.xul.

I think you more mean hiddenWindow.xul. SearchDialog.xul doesn't get loaded on startup.

So it is because mailWindowOverlay.js is being drawn in and that wants a DefaultController because of the newly created inheritance? That feels like we should move it elsewhere, but I won't laden that on you (out of interest, where does SM put it?) - but please change the comment to explicitly say why you're alternately using MessageWindowController even though it isn't ever going to be called like that.

> I also forgot the standalone message window in the first version.

I don't understand - is this just because we got the error opening a standalone message window as well?


You also didn't update specialTabs.js (contentTabType).

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 396495 [details] [diff] [review]
only use existing default controllers

r- based on previous comment.

Comment 5

[Karsten Düsterloh]

Attachment: fix up additional implementations

(In reply to comment #3)
> > drawn into SearchDialog.xul.
> 
> I think you more mean hiddenWindow.xul.

Actually, it's msgAccountCentral.xul, sorry for the confusion.
msgAccountCentral.xul gets loaded on startup and draws mailWindowOverlay.js, which contains mailTabType.

> So it is because mailWindowOverlay.js is being drawn in and that wants a
> DefaultController because of the newly created inheritance?

Yes.

> That feels like we should move it elsewhere, but I won't laden that on you
> (out of interest, where does SM put it?)

Yes, you should put mailTabType into its own file and only include it messenger.xul (and possibly in messageWindow, if you need certain stuff there).
SM defines its mailTabType in a separate tabmail.js file, which gets included by  messenger.xul and messageWindow.xul.

> - but please change the comment to explicitly say why you're alternately
> using MessageWindowController even though it isn't ever going to be called
> like that.

Fixed.

> > I also forgot the standalone message window in the first version.
> 
> I don't understand - is this just because we got the error opening a 
> standalone message window as well?

The standalone window indirectly draws mailTabType as well, but its default controller is named "MessageWindowController", not "DefaultController".
TB doesn't actually seem to use mailTabType in messageWindow.xul, afaict, though, so taking care of it may be bit "overcorrect".

> You also didn't update specialTabs.js (contentTabType).

Fixed.

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 396700 [details] [diff] [review]
fix up additional implementations

Thanks for tidying this up. r=Standard8

Comment 7

[Karsten Düsterloh]

Pushed as <http://hg.mozilla.org/comm-central/rev/67a4f048f573>, finally.