Last Comment Bug 288366 - Port Thunderbird's View Layout to MailNews
: Port Thunderbird's View Layout to MailNews
Status: VERIFIED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Message Display (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Ian Neal
:
Mentors:
Depends on:
Blocks:
  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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
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 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 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 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 neil@parkwaycc.co.uk 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
event.target.childNodes[paneConfig] would work (assuming you passed in the
event).

>+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 -->
>-
:-P
Comment 4 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 event.target.childNodes[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 neil@parkwaycc.co.uk 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"/>
>+</broadcasterset>
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
attribute.
Comment 6 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 neil@parkwaycc.co.uk 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 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 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;
/cvsroot/mozilla/mailnews/base/resources/content/mail3PaneWindowVertLayout.xul,
v  <--	mail3PaneWindowVertLayout.xul
new revision: 1.104; previous revision: 1.103
done
Checking in content/mailWindowOverlay.js;
/cvsroot/mozilla/mailnews/base/resources/content/mailWindowOverlay.js,v  <-- 
mailWindowOverlay.js
new revision: 1.217; previous revision: 1.216
done
Checking in content/mailWindowOverlay.xul;
/cvsroot/mozilla/mailnews/base/resources/content/mailWindowOverlay.xul,v  <-- 
mailWindowOverlay.xul
new revision: 1.290; previous revision: 1.289
done
Checking in content/messageWindow.js;
/cvsroot/mozilla/mailnews/base/resources/content/messageWindow.js,v  <-- 
messageWindow.js
new revision: 1.103; previous revision: 1.102
done
Checking in content/messageWindow.xul;
/cvsroot/mozilla/mailnews/base/resources/content/messageWindow.xul,v  <-- 
messageWindow.xul
new revision: 1.79; previous revision: 1.78
done
Checking in content/messenger.xul;
/cvsroot/mozilla/mailnews/base/resources/content/messenger.xul,v  <-- 
messenger.xul
new revision: 1.256; previous revision: 1.255
done
Checking in content/msgMail3PaneWindow.js;
/cvsroot/mozilla/mailnews/base/resources/content/msgMail3PaneWindow.js,v  <-- 
msgMail3PaneWindow.js
new revision: 1.277; previous revision: 1.276
done
Checking in locale/en-US/messenger.dtd;
/cvsroot/mozilla/mailnews/base/resources/locale/en-US/messenger.dtd,v  <-- 
messenger.dtd
new revision: 1.197; previous revision: 1.196
done
Comment 10 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.