289471 - Add sort by Attachment to mailnews

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, enhancement)

Description

[Ian Neal]

Port TB's sort by attachment to mailnews

Comment 1

[Ian Neal]

Attachment: Provisional Patch v0.1

This patch:
* Adds ability to sort by attachment
* Adds an attachment column to thread pane
* Changes icon for each mail depending on if attachment column is hidden or not

Comment 2

[Ian Neal]

At the moment icons do not consistently update to correct set after changing
state of attachment column. Opening a second 3 pane does show the icons in their
correct new state but still incorrectly in the old one.

Comment 3

[Ian Neal]

Attachment: Update patch v0.1a

Changes since v0.1
* Changed UpgradeThreadPaneUI function to use switch - as suggested by Neil
* Used clearStyleAndImageCaches to update images - as suggested by Neil

Attachment Icons used:
For classic - TB's pinstripe ones (attachment, attachment-col and
attachment-selected)
For modern - TB's qute ones (attachment and attachment-col)

Comment 4

[Ian Neal]

Attachment: mail help patch

This patch:
* Adds a paragraph about how to add/remove columns

Comment 5

[[e-mail]]

Comment on attachment 180125 [details] [diff] [review]
Update patch v0.1a

> function UpgradeThreadPaneUI()
> {
>   try {
>+    var threadPaneUIVersion = pref.getIntPref("mailnews.ui.threadpane.version");
>+    switch (threadPaneUIVersion) {
>+      default: // for all versions
>+        var threadTree = document.getElementById("threadTree");
>+        var junkCol = document.getElementById("junkStatusCol");
>         var subjectCol = document.getElementById("subjectCol");
These won't get set where they are here. In fact the version will always be a
number from 1 to 5. 

>+        var beforeCol;
>+
>+      case 1: // upgrade from 1 to 2
>+        document.getElementById("labelCol").setAttribute("hidden", "true");
It occurs to me that this is unnecessary because at some point I fixed the xul
so that that column already defaults to hidden="true".

>+function UpdateAttachmentCol(addlisten)
Maybe rename this parameter aFirstTimeFlag?

>+    threadTree.setAttribute("attachcol", (hidden != "true") ? "true" : "false");
It would be neat if you could think up a different name for the attribute so
that you could just copy the value. (It's a pity you can't change the name of
an attribute when you observe it using a broadcaster).

>+<!ENTITY sortByAttachmentsCmd.label "Attachments">
>+<!ENTITY sortByAttachmentsCmd.accesskey "m">
>+<!ENTITY attachmentColumn.label "Attachments">
>+<!ENTITY attachmentColumn.tooltip "Click to sort by attachments">
Hmm... aren't these supposed to be listed in the thread pane order?

>+  list-style-image: url("chrome://messenger/skin/icons/attachment-col.png");
Got some pictures of these (preferably all at once in one attachment)? Also
afaik there's some gamma issue with pngs so I'd prefer gifs.

>treechildren::-moz-tree-image(subjectCol) {
>   margin-right: 2px;
>   list-style-image: url("chrome://messenger/skin/icons/message-mail.gif");
> }
> 
> treechildren::-moz-tree-image(subjectCol, new) {
>   list-style-image: url("chrome://messenger/skin/icons/message-mail-new.gif");
> }
> 
>-treechildren::-moz-tree-image(subjectCol, attach) {
>+tree[attachcol="false"] > treechildren::-moz-tree-image(subjectCol, attach) {
>   list-style-image: url("chrome://messenger/skin/icons/message-mail-attach.gif");
> }
> 
>+tree[attachcol="true"] > treechildren::-moz-tree-image(subjectCol, attach) {
>+  list-style-image: url("chrome://messenger/skin/icons/message-mail.gif");
>+}
Now if you look at the earlier rule you'll see that the subjectCol will
probably already want to display the message-mail.gif at this point, except
when you'll end up concealing the display of a new message.

Comment 6

[Ian Neal]

Attachment: Revised patch v0.1b

Changes since v0.1a
* Included mail_help.xhtml fixes
* Removed unnecessary attribute setting from case 1 in UpgradeThreadPaneUI
* Moved code of default case to before switch in UpgradeThreadPaneUI
* Changed case 5 to be the default case in UpgradeThreadPaneUI
* Changed addlisten to aFirstTimeFlag in UpdateAttachmentCol
* Changed attribute "attachcol" to "noattachcol" so it is easier to manipulate
* Moved attachmentCol entities to correct place in .dtd
  (sortBy entities are listed in correct order already)
* Removed unnecessary rules from threadPane.css files
* Changed PNGs to GIFs and included jar.mn file changes

Comment 7

[Ian Neal]

Attachment: All new GIFs in one GIF

Comment 8

[Ian Neal]

Attachment: Actual new GIFs in zipfile

So they can be seen actual size

Comment 9

[Ian Neal]

Attachment: new GIFs v0.1b1

New GIFs for classic from Neil

Comment 10

[Ian Neal]

Attachment: Revised all in one GIF of revised GIFs

Includes new GIFs from Neil for Classic

Comment 11

[[e-mail]]

Comment on attachment 180296 [details] [diff] [review]
Revised patch v0.1b

>+function UpdateAttachmentCol(aFirstTimeFlag)
>+{
>+    var attachmentCol = document.getElementById("attachmentCol");
>+    var threadTree = GetThreadTree();
>+    threadTree.setAttribute("noattachcol", attachmentCol.getAttribute("hidden"));
>+    if (aFirstTimeFlag)
>+      attachmentCol.addEventListener("DOMAttrModified", OnAttachmentColAttrModified, false);
>+    else
>+      threadTree.treeBoxObject.clearStyleAndImageCaches();
> }
Mixed 2- and 4-space indents :-( Please use 2-space indents for new functions.

>+  padding-right: 5px; 
I don't see the point of this padding.

r=me with these nits fixed.

Comment 12

[Ian Neal]

Attachment: Updated Patch v0.1c (Checked in)

Changes since patch v0.1b:
* Fixed mixed indents as per review
* Removed unneeded padding-right: 5px as per review

Carrying forward r= and requesting sr=

Comment 13

[Ian Neal]

Comment on attachment 182972 [details] [diff] [review]
Updated Patch v0.1c (Checked in)

Requesting approval for a suite only patch which is fairly low risk.

Comment 14

[Asa Dotzler [:asa]]

Comment on attachment 182972 [details] [diff] [review]
Updated Patch v0.1c (Checked in)

a=asa

Comment 15

[Ian Neal]

Comment on attachment 182972 [details] [diff] [review]
Updated Patch v0.1c (Checked in)

Checking in mailnews/base/resources/content/commandglue.js;
new revision: 1.256; previous revision: 1.255
Checking in mailnews/base/resources/content/mailWindowOverlay.js;
new revision: 1.218; previous revision: 1.217
Checking in mailnews/base/resources/content/mailWindowOverlay.xul;
new revision: 1.294; previous revision: 1.293
Checking in mailnews/base/resources/content/msgMail3PaneWindow.js;
new revision: 1.279; previous revision: 1.278
Checking in mailnews/base/resources/content/threadPane.xul;
new revision: 1.132; previous revision: 1.131
Checking in mailnews/base/resources/locale/en-US/messenger.dtd;
new revision: 1.199; previous revision: 1.198
Checking in mailnews/base/resources/locale/en-US/threadpane.dtd;
new revision: 1.25; previous revision: 1.24
Checking in themes/classic/jar.mn;
new revision: 1.133; previous revision: 1.132
Checking in themes/classic/messenger/threadPane.css;
new revision: 1.22; previous revision: 1.21
RCS file: /cvsroot/mozilla/themes/classic/messenger/icons/attachment.gif,v
Checking in themes/classic/messenger/icons/attachment.gif;
initial revision: 1.1
RCS file: /cvsroot/mozilla/themes/classic/messenger/icons/attachment-col.gif,v
Checking in themes/classic/messenger/icons/attachment-col.gif;
initial revision: 1.1
RCS file:
/cvsroot/mozilla/themes/classic/messenger/icons/attachment-selected.gif,v
Checking in themes/classic/messenger/icons/attachment-selected.gif;
initial revision: 1.1
Checking in themes/modern/jar.mn;
new revision: 1.147; previous revision: 1.146
Checking in themes/modern/messenger/threadPane.css;
new revision: 1.42; previous revision: 1.41
RCS file: /cvsroot/mozilla/themes/modern/messenger/icons/attachment.gif,v
Checking in themes/modern/messenger/icons/attachment.gif;
initial revision: 1.1
RCS file: /cvsroot/mozilla/themes/modern/messenger/icons/attachment-col.gif,v
Checking in themes/modern/messenger/icons/attachment-col.gif;
initial revision: 1.1
RCS file:
/cvsroot/mozilla/themes/modern/messenger/icons/attachment-selected.gif,v
Checking in themes/modern/messenger/icons/attachment-selected.gif;
initial revision: 1.1
done

Comment 16

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using both Classic and Modern themes on Windows XP Seamonkey
trunk build 2005-05-16-06.