Closed Bug 76216 Opened 23 years ago Closed 23 years ago

Search UI: default results columns not as spec'd

Categories

(SeaMonkey :: MailNews: Message Display, defect, P3)

Product:
SeaMonkey
Component:
MailNews: Message Display
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.1

People

(Reporter: laurel, Assigned: stephend)

References

Details

(Whiteboard: [nsbeta1+])

Attachments

(6 files)

Patch
1.71 KB, patch
Details | Diff | Splinter Review
a patch to get stephend started.
2.17 KB, patch
Details | Diff | Splinter Review
Patch.
2.65 KB, patch
Details | Diff | Splinter Review
Patch, doesn't call ShowSearchColumn() on items that appear by themselves.
4.00 KB, patch
Details | Diff | Splinter Review
Path, stops hiding the priority column.
3.97 KB, patch
Details | Diff | Splinter Review
Patch
4.09 KB, patch
Details | Diff | Splinter Review 
Carried forward from bug 44341
spec: http://www.mozilla.org/mailnews/specs/search/

Default search messages results pane columns are still not according to spec : 
"The default columns (and order) in the Results area should be: Thread, Subject,
Sender, Date, Location, Column Widget."

We know thread column is out (bug 72253), but still have different and more
columns than defaults show per spec.

Apr 16 commercial build displays these default columns (too many):
Subject, Sender, Read/Unread, Date, Status, Size, Flag, Priority, Location,
column widget.
QA Contact: esther → laurel
gayatrib, feel free to take this from me.

it should be an easy fix.  see SearchDialog.js for where I set the hidden and
ignoreincolumn picker attributes on certain columns.
re-assign to gayatri
Assignee: sspitzer → gayatrib
I tested this in my debug build on Win2K, worked like a charm.  Please
review/sr.  Thanks.
r=gayatrib
looks cool. thanks stephend.
you've hidden the columns, but they will still show in the column picker which
is incorrect.  

why'd you remove the "ignoreincolumn" code?

let's take this opportunity to clean up the code.  implement a hideColumn() and
a showColumn() that take an id and does the right thing.  that should clean up
our code and remove all the copy and paste.
 
I was clear on which columns should show up by default, but I didn't realize
that we were supposed to hide the rest of the columns.  Sorry Seth, but I can't
implement those functions you mention.
I will take the bug back.
stephend, here comes a patch to get you started.
marking nsbeta1+ and moving to mozilla0.9.1
Priority: -- → P3
Whiteboard: [nsbeta1+]
Target Milestone: --- → mozilla0.9.1
Thanks for the start, Seth.  It was simple to get going after that.  A question
for Jennifer remains before I post the patch.  Looking at
http://www.mozilla.org/mailnews/specs/search/, under "Results", it lists the
defaults for the column picker widget that Laurel filed this bug against. 
However, Seth says, "You've hidden the columns, but they will still show in the
column picker which is incorrect."  Am I to understand that this Search Messages
dialog is going to be widdled down to (possibly) 4 displayable results? Don't we
want the whole slew of columns in there, just not visible by default? If not, my
patch works and I'll attach it. If we need them (except for thread) to be
available in the column picker but just not enabled by default, then I'll need
to do write a function such as:

function HideButShowInSearchColumnPicker(id)
{
  var col = document.getElementById(id);
  if (col) {
    col.setAttribute("hidden", "true");
    col.setAttribute("ignoreincolumnpicker", "false");
  }

4.x had at most five columns:  sender, subject, date, priority and location.

I think we want 6.x to be the same.  [there's probably a good reason to not
allow the user to change the flag or unread state of a message from the search
dialog.]

in which case, we want to hide them and not show them in the picker.

like 4.x, we the use can always hide some of the five.

note:  thread column, unread total in thread column, and total in thread column
should never be shown, since they are thread specific and have no value unless
you are in threaded mode.

HideButShowInSearchColumnPicker() is not needed.

if you attach a complete patch, we can get it reviewed and in for 0.9.1 (or 0.9
if you ask drivers@mozilla.org really nice)
Note: the sender column always seems enabled (can't change the attribute of it), 
but this also occurs in the thread pane (which are where these are inherited 
from).
My initial thought was that only certain columns would be displayed by default, 
but still be available from the column picker. But Seth brings up a good point 
of not having columns available that don't server a purpose, such as the thread 
specific columns.  

If we allowed the flag or read/unread column, and users changed the values in 
the search dialog, they would have to be changed in 3 pane as well. Not sure how 
difficult that is to do or if we want to allow it. Seth/Stephen, whatever you 
think is best on this one if fine with me.
My vote, at least for this release, is to not allow the unread or flag column. 
Because it would need tracking to the other mail window(s), I can only surmise
there would be a lot of potential bugs there. We have a plethora of search bugs
on the list already which we don't have time for, I don't think we should
intentionally add to it for this release timeframe.
Sounds good.
Okay, my patch uses Seth's functions, and enables only the columns that Laurel
refers to from Jennifer's spec.  I looked at the 4.x widget, and we're exactly
the same (with Seth's/my) patch, except 4.x also had Priority.  But since the
spec leaves that out, so does my patch, and it looks like this is all ready for
the r/sr stages.  Also, Seth, when/if I checkin, I'll remove the setAttribute
for the Sender column, since that doesn't actually have an affect.
sr=mscott, but please get an r= from gayatri before checking in. She's the
module owner for search.
Gayatri, please re-review the new functions and the calls to these functions. 
I'd like to try getting this into 0.9, if possible.  Thanks.  
no go on this patch.

+  // we want to show the location column for search
+  ShowSearchColumn("locationCol");

you don't need to show the other columns.  you only need to show the columns
that are hidden by default, which is the location column.

please attach a new patch.

I'd argue to make show the priority column in the search dialog.

weird, we have a picker for priority ("Lowest", "Low", "Normal", "High",
"Highest") but no way to search on priority.

I'm unable to search for "priority is high".  that's a bug, I'll log it.


spun off bug #77181 (on priority searching)
Depends on: 77232
No longer depends on: 77232
Blocks: 77232
the screen shot in the spec has a priority column.

can you fix the patch to not hide / remove the priority column?
flaggedCol is listed twice.

how about something like this:

+  // hide and remove these columns from the column picker.  
+  HideSearchColumn("threadCol"); // since you can't thread search results
+  HideSearchColumn("totalCol"); // since you can't thread search results
+  HideSearchColumn("unreadCol"); // since you can't thread search results
+  HideSearchColumn("unreadButtonColHeader");
+  HideSearchColumn("flaggedCol");
+  HideSearchColumn("statusCol");
+  HideSearchColumn("sizeCol");



after gayatrib reviews, I can super review.

stephend, you should take this bug.
mine all mine
Assignee: gayatrib → stephend
Status: NEW → ASSIGNED
r=gayatrib. thank you stephend.
Thanks to all for reviewing.  Fix checked in!
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
OK with apr27 commercial trunk build: win98, Mac OS 9.0, linux rh6.2
Status: RESOLVED → VERIFIED
I strongly disagree with the other columns not even being available. Filed bug
99995.
Product: Browser → Seamonkey
Component: MailNews: Search → MailNews: Message Display
QA Contact: laurel → search
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: