Search UI: default results columns not as spec'd

VERIFIED FIXED in mozilla0.9.1

Status

SeaMonkey
MailNews: Message Display
P3
normal
VERIFIED FIXED
17 years ago
10 years ago

People

(Reporter: laurel, Assigned: stephend@netscape.com (gone - use stephen.donner@gmail.com instead))

Tracking

Trunk
mozilla0.9.1

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta1+])

Attachments

(6 attachments)

(Reporter)

Description

17 years ago
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.
(Reporter)

Updated

17 years ago
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
Keywords: nsbeta1, patch, review, ui
I tested this in my debug build on Win2K, worked like a charm.  Please
review/sr.  Thanks.

Comment 5

17 years ago
r=gayatrib

Comment 6

17 years ago
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.

Comment 9

17 years ago
I will take the bug back.
stephend, here comes a patch to get you started.

Comment 12

17 years ago
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).

Comment 17

17 years ago
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.
(Reporter)

Comment 18

17 years ago
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.

Comment 19

17 years ago
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.

Comment 21

17 years ago
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)
Created attachment 31867 [details] [diff] [review]
Patch, doesn't call ShowSearchColumn() on items that appear by themselves.
(Reporter)

Updated

17 years ago
Depends on: 77232
(Reporter)

Updated

17 years ago
No longer depends on: 77232
(Reporter)

Updated

17 years ago
Blocks: 77232
the screen shot in the spec has a priority column.

can you fix the patch to not hide / remove the priority column?
Created attachment 31961 [details] [diff] [review]
Path, stops hiding 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

Comment 33

17 years ago
r=gayatrib. thank you stephend.
Thanks to all for reviewing.  Fix checked in!
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
(Reporter)

Comment 36

17 years ago
OK with apr27 commercial trunk build: win98, Mac OS 9.0, linux rh6.2
Status: RESOLVED → VERIFIED

Comment 37

17 years ago
I strongly disagree with the other columns not even being available. Filed bug
99995.
Product: Browser → Seamonkey

Updated

10 years ago
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.