Make quick search view a subclass of nsMsgDBView

VERIFIED FIXED

Status

VERIFIED FIXED
16 years ago
10 years ago

People

(Reporter: naving, Assigned: naving)

Tracking

Trunk
x86
Windows 2000

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

16 years ago
 

Comment 1

16 years ago
does threading work in quick search? I would think it would be a subclass of
nsMsgDBView, unless there's some reason that doesn't work.
(Assignee)

Comment 2

16 years ago
I'll have to see.
Summary: Make quick search view a subclass of nsMsgThreadedDBView → Make quick search view a subclass of nsMsgDBView
(Assignee)

Comment 3

16 years ago
Created attachment 96514 [details] [diff] [review]
proposed patch

I made nsMsgQuickSearchDBView a subclass of nsMsgDBView and now we are creating
a new quick search view when we do quick search. We cache the old folder view
and once the user comes out of quick search we just set that view. All other
changes
should be self explanatory.
(Assignee)

Comment 4

16 years ago
Cavin, David, Can you review ? thx.
Status: NEW → ASSIGNED
(Assignee)

Comment 5

16 years ago
Created attachment 96712 [details] [diff] [review]
proposed patch

fixed some selection issues. please review, thx.
Attachment #96514 - Attachment is obsolete: true

Comment 6

16 years ago
Comment on attachment 96712 [details] [diff] [review]
proposed patch

r=cavin.
Attachment #96712 - Flags: review+

Comment 7

16 years ago
It occurs to me that you should talk to mscott about his view work to make sure
he can work with this change...I'm not sure exactly what his plans are for
landing on the trunk, if any. I'll go ahead and do the review, but I'd
appreciate it if you talked to him before checking in, if you haven't already.

Comment 8

16 years ago
I notice you're only restoring single selection - is there a reason you don't
just save away the whole selection? Is that the way this used to work?

Are you disabling Expand and CollapseAll in the search results view? It looks
like the old code used to do that.

nit:
NS_ASSERTION(0 should be NS_ASSERTION(PR_FALSE - it's just slightly more readable

Overall, I'm very happy that this is its own view. It makes the code much cleaner.
(Assignee)

Comment 9

16 years ago
>I notice you're only restoring single selection - is there a reason you don't
>just save away the whole selection? Is that the way this used to work?

yes, I'm restoring single selection because I think it is more than sufficient
because selecting a bunch of msgs in search view and then coming out of search,
it won't be contigous anyway and also there was no existing scriptable method
that I could use. 

>Are you disabling Expand and CollapseAll in the search results view? It looks
>like the old code used to do that.

this was not disabled previously but we were preventing these commands to be 
executed in nsMsgDBView.cpp. So disabling them makes more sense. 

I'll talk to mscott. I wonder why he is not responding on aim!

Comment 10

16 years ago
one other comment - instead of checking for the quick search view type in
multiple places, can you add a readonly bool attribute to nsIMsgDBView.idl,
supportsThreading - implement it as PR_FALSE in nsMsgDBView.cpp and PR_TRUE in
nsMsgThreadedDBView.  We very well might have other views that didn't support
threading. So, then we'd check this boolean when we disable the thread icon/menu
and the expand/collapse command, among others.
(Assignee)

Comment 11

16 years ago
Created attachment 96887 [details] [diff] [review]
patch w/ GetSupportsThreading
(Assignee)

Updated

16 years ago
Attachment #96712 - Attachment is obsolete: true

Updated

16 years ago
Attachment #96887 - Flags: superreview+

Comment 12

16 years ago
Comment on attachment 96887 [details] [diff] [review]
patch w/ GetSupportsThreading

thx, great, sr=bienvenu
(Assignee)

Comment 13

16 years ago
fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
I hate to ask, but LXR-verifying that this change was checked in is okay, right?

Laurel's been testing search for a while, and I'm sure she's logged any
regressions that might have resulted from this fix.
QA Contact: gayatri → stephend

Comment 15

16 years ago
yes, lxr verification is fine.

Comment 16

16 years ago
I agree - thanks for verifying this!
Verified FIXED.

All files were checked in on August 29th, 2002.
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.