Last Comment Bug 288366 - Port Thunderbird's View Layout to MailNews
: Port Thunderbird's View Layout to MailNews
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Message Display (show other bugs)
: Trunk
: All All
-- normal (vote)
: ---
Assigned To: Ian Neal
Depends on:
  Show dependency treegraph
Reported: 2005-03-30 15:08 PST by Ian Neal
Modified: 2006-01-04 14:08 PST (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

Provisional patch v0.1 (14.21 KB, patch)
2005-03-30 15:32 PST, Ian Neal
no flags Details | Diff | Splinter Review
Tweaked Patch v0.1a (9.31 KB, patch)
2005-03-30 16:10 PST, Ian Neal
neil: review-
Details | Diff | Splinter Review
Revised Patch v0.1c (24.06 KB, patch)
2005-04-01 17:41 PST, Ian Neal
no flags Details | Diff | Splinter Review
Extended Patch v0.1e (62.39 KB, patch)
2005-04-04 17:42 PDT, Ian Neal
neil: review+
Details | Diff | Splinter Review
Tweaked extended patch v0.1f (Checked in) (62.40 KB, patch)
2005-04-05 09:19 PDT, Ian Neal
iann_bugzilla: review+
mozilla: superreview+
Details | Diff | Splinter Review

Description User image Ian Neal 2005-03-30 15:08:27 PST
Back on 2003-08-11 20:12 Thunderbird got a Layout switcher under its View menu,
it would be nice to port that.
Comment 1 User image Ian Neal 2005-03-30 15:32:14 PST
Created attachment 179114 [details] [diff] [review]
Provisional patch v0.1

This patch does the equivalent of the one that was checked in for TB in 2003.
Comment 2 User image Ian Neal 2005-03-30 16:10:04 PST
Created attachment 179121 [details] [diff] [review]
Tweaked Patch v0.1a

Changes from v0.1
* Only adds strings to messages.dtd leaves unimplemented/obsolete strings alone
Comment 3 User image 2005-04-01 06:01:56 PST
Comment on attachment 179121 [details] [diff] [review]
Tweaked Patch v0.1a

>+function InitViewLayoutStyleMenu()
>+  var paneConfig = pref.getIntPref("mail.pane_config.dynamic");
Old profiles don't have this preference. Use GetMailPaneConfig() instead.

>+  switch (paneConfig) 
>+  {
>+    case kClassicMailLayout:
>+      id ="messagePaneClassic";
>+      break;
>+    case kWideMailLayout:	
>+      id = "messagePaneWide";
>+      break;
>+    case kVerticalMailLayout:
>+      id = "messagePaneVertical";
>+      break;
>+  }
>+  var layoutStyleMenuitem = document.getElementById(id);
Well this is a bit of an ugly way of doing this. One alternative would be to
put the ids in an array i.e. ["messagePaneClassic", "messagePaneWide",
"messagePaneVertical"] or another alternative would be to note that the
menuitems are the first three items in the popup so that[paneConfig] would work (assuming you passed in the

>+function ChangeMailLayout(newLayout)
>+  gPrefBranch.setIntPref("mail.pane_config.dynamic", newLayout);
>+  return true;
You don't actually need to return true here.

>-          <menuseparator id="menu_showSearch_showMessage_Separator"/>
You didn't remove the code in HideMenus that refers to this.

>-          <menuitem id="menu_showMessagePane"
>-           type="checkbox"
>-           label="&showMessagePaneCmd.label;"
>-           accesskey="&showMessagePaneCmd.accesskey;"
>-           key="key_toggleMessagePane"
>-           oncommand="MsgToggleMessagePane();"/>
>-          <menuitem id="menu_showFolderPane"
>-           type="checkbox"
>-           label="&showFolderPaneCmd.label;"
>-           accesskey="&showFolderPaneCmd.accesskey;"
>-           key="key_toggleFolderPane"
>-           oncommand="MsgToggleSplitter('gray_vertical_splitter');"/>
Now you're moving these to a menu that you're already hiding you can remove the
code in HideMenus that hides them.

>+  var viewLayoutMenu = document.getElementById("menu_MessagePaneLayout");
>+  if (viewLayoutMenu)
>+    viewLayoutMenu.setAttribute("hidden", "true");
[Aside: if all these menuitems observed a broadcaster we could set them all to
hidden in XUL instead]

> <!-- Go Menu -->
Comment 4 User image Ian Neal 2005-04-01 17:41:55 PST
Created attachment 179339 [details] [diff] [review]
Revised Patch v0.1c

Changes since v0.1a
* Made use of GetMailPaneConfig() and[paneConfig] for
layout switching plus other tidy-ups as suggested by Neil
* Used a broadcaster to hide menus as suggested by Neil
* Removes unused function UpdateMailPaneConfigMenu from msgMail3PaneWindow.js
as suggested by Neil on IRC

There is possible scope for using another broadcaster for disabled menus when
account manager is open - see view_init function in mailWindowOverlay.js - but
that is probably for another bug
Comment 5 User image 2005-04-04 05:03:55 PDT
Comment on attachment 179339 [details] [diff] [review]
Revised Patch v0.1c

>+function ChangeMailLayout(newLayout)
>+  gPrefBranch.setIntPref("mail.pane_config.dynamic", newLayout);
>+  return;
What I meant was that the return statement was unnecessary...

>+<broadcasterset id="mailBroadcasters">
>+  <broadcaster id="mailHideMenus" hidden="false"/>
You shouldn't need to specify hidden here as it gets copied to all the
menuitems which you don't need for the 3 pane views. It might even be neater to
specify the broadcaster with hidden="true" in messageWindow.xul and without
hidden in the two 3 pane files.

>+                observes="cmd_renameFolder">
>+        <observes element="mailHideMenus" attribute="hidden"/>
In theory we should be able to switch these menuitems to command= which is
cheaper for menuitems and keys, and also allows you to use the observes
Comment 6 User image Ian Neal 2005-04-04 17:42:57 PDT
Created attachment 179636 [details] [diff] [review]
Extended Patch v0.1e

Changes since v0.1c
* Removed return statement from ChangeMailLayout function
* Moved broadcaster out of mailWindowOverlay.xul and into the 3 views
* Added a new broadcaster to disable F8/F9 keys in messageWindow.xul
  - gets rid of MsgToggleMessagePane and MsgToggleSplitter errors in JS console

* Change all observes="cmd..." to command="cmd..."
* Change all <observes element="mail..." into simple observes="mail..."
Comment 7 User image 2005-04-05 05:51:05 PDT
Comment on attachment 179636 [details] [diff] [review]
Extended Patch v0.1e

>+  <broadcaster id="mailHideKeys"/>
Disable :-P
Comment 8 User image Ian Neal 2005-04-05 09:19:46 PDT
Created attachment 179717 [details] [diff] [review]
Tweaked extended patch v0.1f (Checked in)

Changes since v0.1e
* Changed broadcaster name from mailHideKeys to mailDisableKeys as suggested by
Neil (I think)

Carrying forward r=
Comment 9 User image Ian Neal 2005-04-05 09:58:36 PDT
Comment on attachment 179717 [details] [diff] [review]
Tweaked extended patch v0.1f (Checked in)

Checking in content/mail3PaneWindowVertLayout.xul;
v  <--	mail3PaneWindowVertLayout.xul
new revision: 1.104; previous revision: 1.103
Checking in content/mailWindowOverlay.js;
/cvsroot/mozilla/mailnews/base/resources/content/mailWindowOverlay.js,v  <-- 
new revision: 1.217; previous revision: 1.216
Checking in content/mailWindowOverlay.xul;
/cvsroot/mozilla/mailnews/base/resources/content/mailWindowOverlay.xul,v  <-- 
new revision: 1.290; previous revision: 1.289
Checking in content/messageWindow.js;
/cvsroot/mozilla/mailnews/base/resources/content/messageWindow.js,v  <-- 
new revision: 1.103; previous revision: 1.102
Checking in content/messageWindow.xul;
/cvsroot/mozilla/mailnews/base/resources/content/messageWindow.xul,v  <-- 
new revision: 1.79; previous revision: 1.78
Checking in content/messenger.xul;
/cvsroot/mozilla/mailnews/base/resources/content/messenger.xul,v  <-- 
new revision: 1.256; previous revision: 1.255
Checking in content/msgMail3PaneWindow.js;
/cvsroot/mozilla/mailnews/base/resources/content/msgMail3PaneWindow.js,v  <-- 
new revision: 1.277; previous revision: 1.276
Checking in locale/en-US/messenger.dtd;
/cvsroot/mozilla/mailnews/base/resources/locale/en-US/messenger.dtd,v  <-- 
new revision: 1.197; previous revision: 1.196
Comment 10 User image Stephen Donner [:stephend] 2006-01-04 14:08:42 PST
Verified FIXED using trunk build 2006-01-4-09 under Windows XP.

I checked; all 3 layouts (Classic, Wide, Vertical) are present and work correctly, as do the Message Pane and Folder Pane toggle options.

Note You need to log in before you can comment on or make changes to this bug.