Tabbed Composition - implementation questions

[Liam Edwards-Playne]

Hello all,

As you may/may not know, I've been working on tabbed composition for the past month now and I'd like some help on the codebase/architecture. Currently ported the bulk of OpenComposeWindow and OpenComposeWindowParams to JS functions and have rewritten nearly all the calls to the JS port, and now looking into the window/tab management stuff.  

As discussed on BugZilla, it would be best to implement the new 'compose tab' like the message view tab (mail/base/content/mailTabs.js), so it can be opened in either a new window or new tab and so forth. This would also allow for older users to still use windows (should tabbed composition be a default?). So how are they implemented? How can I iterate over open compose tabs etc.?

After this, I have to finish porting OpenComposeWindow, which includes:

- integrating GetOrigWindowSelection to use the new compose tab mode

- integrating LoadDraftOrTemplate - which is a tricky function compared to the others because it goes down a long road of function calls (DisplayMessage onwards) before it reaches any UI-related code. 

- integrating ComposeMessage to iterate through open draft tabs

- rewrite the argument passing mechanism (nsMsgComposeParams) to use something other than window.arguments - MsgComposeCommands.js

- some other misc. stuff todo with js string encoding

After that, some more things:

- Rewrite the code to close the message tab (so it doesn't try to close the window and crash)

- Redesign the UX so there are not 2 menu bars - this will be very interesting

- Add options similar to "Preferences->Advanced->Reading and Display" (open message in new tab/window etc.)

When the majority of the coding is done, we'll have to get in touch with UX for the menu bar and whether compose should open in a new tab by default. 

Look forward to getting this long-awaited feature done.

Liam (liamzebedee) Edwards-Playne

[Ludovic Hirlimann]

On 3/1/13 7:26 AM, Liam Edwards-Playne wrote:

After this, I have to finish porting OpenComposeWindow, which includes:

- integrating GetOrigWindowSelection to use the new compose tab mode

- integrating LoadDraftOrTemplate - which is a tricky function compared to the others because it goes down a long road of function calls (DisplayMessage onwards) before it reaches any UI-related code. 

- integrating ComposeMessage to iterate through open draft tabs

- rewrite the argument passing mechanism (nsMsgComposeParams) to use something other than window.arguments - MsgComposeCommands.js

- some other misc. stuff todo with js string encoding

After that, some more things:

- Rewrite the code to close the message tab (so it doesn't try to close the window and crash)

- Redesign the UX so there are not 2 menu bars - this will be very interesting

- Add options similar to "Preferences->Advanced->Reading and Display" (open message in new tab/window etc.)

Can you also add - add tests tests tests and more tests ?

Ludo

-- 
SRE Team
QA Lead for Thunderbird
http://sietch-tabr.tumblr.com/

[Mike Conley]

Hey Liam,

This is great news! We've wanted tabbed composition for some time, and
I'm glad to hear somebody is hacking on it!

>> As discussed on BugZilla

Bug #?

> So how are they implemented? How can I
>> iterate over open compose tabs etc.?

You've already identified where most of our tab types are defined:

http://mxr.mozilla.org/comm-central/source/mail/base/content/mailTabs.js

There are more examples here:

http://mxr.mozilla.org/comm-central/source/mail/base/content/specialTabs.js

The tabmail binding that we use is pretty complex - but it's also
reasonably documented.

I would give this a thorough read:

http://mxr.mozilla.org/comm-central/source/mail/base/content/tabmail.xml#18

I can also do my best to answer specific questions about tabs - either
send me mail, or find me in IRC (mconley in #maildev).

> After this, I have to finish porting OpenComposeWindow

This is an excellent opportunity for you examine our implementation of
compose windows and finding out where we can optimize, and where we can
clean up. Almost certainly there are things we do with the current
compose code that is no longer necessary, flat-out confusing, or just
generally suboptimal. This makes maintenance a heavy burden. Keep an eye
out for those - this is an excellent opportunity to clean some of those
things up. I encourage aggressive refactoring.

Josiah Bruner has been doing some excellent work coming up with ways of
cleaning up our UI[1]. At some point, perhaps yourself, Josiah, and a
few other UI-involved folks should sit down and hash out what a new
compose tab would look like.

Anyhow, I'm thoroughly excited. Please send me the bug number you're
working in so I can monitor your progress.

Thanks,

-Mike

[1]: http://infinite-josiah.blogspot.ca/2013/02/thunderbird-ui-concept.html

> _______________________________________________
> tb-planning mailing list
> tb-pl...@mozilla.org
> https://mail.mozilla.org/listinfo/tb-planning
>
_______________________________________________
tb-planning mailing list
tb-pl...@mozilla.org
https://mail.mozilla.org/listinfo/tb-planning

[Michel Renon]

Hi Liam,

Le 01/03/2013 07:26, Liam Edwards-Playne a écrit :
> Hello all,
>
> As you may/may not know, I've been working on tabbed composition for the
> past month now and I'd like some help on the codebase/architecture.

> [...]

> When the majority of the coding is done, we'll have to get in touch with
> UX for the menu bar and whether compose should open in a new tab by
> default.
>
> Look forward to getting this long-awaited feature done.

Still on the UX side, have you seen a previous addon by Jonathan Protzenko ?
https://addons.mozilla.org/fr/thunderbird/addon/compose-for-thunderbird

As I was interested by the address list panel, I found that it used the
'loopj' jquery plugin (http://loopj.com/jquery-tokeninput/)
Unfortunately, it doesn't work anymore since approx. one year.
FYI, I rewrote a complete addon to perform such a behavior :
https://addons.mozilla.org/fr/thunderbird/addon/mrc-compose/
It doesn't provide tokens (recipients as blue rectangle) but everything
else is ok.
Feel free to reuse as much code as you want. And I'll be happy to answer
any question about it.

Thanks,
Michel

[Jonathan Protzenko]

Hi Liam,

Sorry for the delay in replying. I've been updating the bug with a comment, but I'm replying here as well for posterity.

On 03/01/2013 07:26 AM, Liam Edwards-Playne wrote:

Hello all,

As you may/may not know, I've been working on tabbed composition for the past month now and I'd like some help on the codebase/architecture. Currently ported the bulk of OpenComposeWindow and OpenComposeWindowParams to JS functions and have rewritten nearly all the calls to the JS port, and now looking into the window/tab management stuff.  

As discussed on BugZilla, it would be best to implement the new 'compose tab' like the message view tab (mail/base/content/mailTabs.js), so it can be opened in either a new window or new tab and so forth. This would also allow for older users to still use windows (should tabbed composition be a default?). So how are they implemented? How can I iterate over open compose tabs etc.?

The bulk of the code that does tab handling is in http://mxr.mozilla.org/comm-central/source/mail/base/content/tabmail.xml . You can iterate over tabs by doing (untested):

let tabmail = document.getElementById("tabmail");
for (let candidate of tabmail.tabInfo)
  dump(candidate.mode);

You can easily define new tab types, I'm pretty sure that's what I did with the "compose in a tab" experiment. You have to reimplement a few things yourself but specialtabs.js contains good examples of how to implement a new tab type.

After this, I have to finish porting OpenComposeWindow, which includes:

- integrating GetOrigWindowSelection to use the new compose tab mode

- integrating LoadDraftOrTemplate - which is a tricky function compared to the others because it goes down a long road of function calls (DisplayMessage onwards) before it reaches any UI-related code. 

- integrating ComposeMessage to iterate through open draft tabs

- rewrite the argument passing mechanism (nsMsgComposeParams) to use something other than window.arguments - MsgComposeCommands.js

- some other misc. stuff todo with js string encoding

I would suggest, as much as you can, to break this down into smaller patches. You could implement, as a first step, a "composition manager" written in JS. The JS object would provide JS-implemented versions of GetOrigWindowSelection, possibly LoadDraftOrTemplate. This would simplify the code by a significant margin, while providing after that an easy way to build on top of that JS and make it work with composition in a tab. Iterating of the tabs from C++, inside GetOrigWindowSelection is going to be an incredible pain -- I think it's worth looking into switching some of these methods to JS first.

After that, some more things:

- Rewrite the code to close the message tab (so it doesn't try to close the window and crash)

I've had that as well!

- Redesign the UX so there are not 2 menu bars - this will be very interesting

This is another big chunk of work. What about having the "composition in a tab" feature pref'd off, while we work out to solve these issues? This would allow you to land a first set of patches and prevent them from bitrotting, and this would allow other people to start playing with the feature and find bugs, while we solve other problems in smaller chunks.

- Add options similar to "Preferences->Advanced->Reading and Display" (open message in new tab/window etc.)

When the majority of the coding is done, we'll have to get in touch with UX for the menu bar and whether compose should open in a new tab by default. 

Look forward to getting this long-awaited feature done.

As Magnus said on the bug, it's usually better to hang out in #maildev on IRC as people tend to be more responsive there.

Good luck and don't hesitate to ask specific questions about the code in this thread, I'll do my best to reply if I have the knowledge.

Cheers,

jonathan