Closed Bug 289471 Opened 19 years ago Closed 19 years ago

Add sort by Attachment to mailnews

Categories

(SeaMonkey :: MailNews: Message Display, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: iannbugzilla, Assigned: iannbugzilla)

Details

Attachments

(3 files, 6 obsolete files)

Port TB's sort by attachment to mailnews
Attached patch Provisional Patch v0.1 (obsolete) — Splinter Review
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
Assignee: sspitzer → bugzilla
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.
Status: NEW → ASSIGNED
Attached patch Update patch v0.1a (obsolete) — Splinter Review
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)
Attachment #180000 - Attachment is obsolete: true
Attachment #180125 - Flags: review?(neil.parkwaycc.co.uk)
Attached patch mail help patch (obsolete) — Splinter Review
This patch:
* Adds a paragraph about how to add/remove columns
Attachment #180185 - Flags: review?(neil.parkwaycc.co.uk)
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.
Attachment #180125 - Flags: review?(neil.parkwaycc.co.uk) → review-
Attachment #180185 - Flags: review?(neil.parkwaycc.co.uk)
Attached patch Revised patch v0.1b (obsolete) — Splinter Review
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
Attachment #180125 - Attachment is obsolete: true
Attachment #180185 - Attachment is obsolete: true
Attachment #180296 - Flags: review?(neil.parkwaycc.co.uk)
Attached image All new GIFs in one GIF (obsolete) —
Attached file Actual new GIFs in zipfile (obsolete) —
So they can be seen actual size
Attached file new GIFs v0.1b1
New GIFs for classic from Neil
Attachment #180298 - Attachment is obsolete: true
Includes new GIFs from Neil for Classic
Attachment #180297 - Attachment is obsolete: true
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.
Attachment #180296 - Flags: review?(neil.parkwaycc.co.uk) → review+
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=
Attachment #180296 - Attachment is obsolete: true
Attachment #182972 - Flags: superreview?(bienvenu)
Attachment #182972 - Flags: review+
Attachment #182972 - Flags: superreview?(bienvenu) → superreview+
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.
Attachment #182972 - Flags: approval1.8b2?
Comment on attachment 182972 [details] [diff] [review]
Updated Patch v0.1c (Checked in)

a=asa
Attachment #182972 - Flags: approval1.8b2? → approval1.8b2+
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
Attachment #182972 - Attachment description: Updated Patch v0.1c → Updated Patch v0.1c (Checked in)
Severity: normal → enhancement
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Verified FIXED using both Classic and Modern themes on Windows XP Seamonkey
trunk build 2005-05-16-06.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: