163773 - Make quick search view a subclass of nsMsgDBView

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[Navin Gupta]

Comment 1

[David :Bienvenu]

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.

Comment 2

[Navin Gupta]

I'll have to see.

Comment 3

[Navin Gupta]

Attachment: 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.

Comment 4

[Navin Gupta]

Cavin, David, Can you review ? thx.

Comment 5

[Navin Gupta]

Attachment: proposed patch

fixed some selection issues. please review, thx.

Comment 6

[Cavin Song]

Comment on attachment 96712 [details] [diff] [review]
proposed patch

r=cavin.

Comment 7

[David :Bienvenu]

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

[David :Bienvenu]

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.

Comment 9

[Navin Gupta]

>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

[David :Bienvenu]

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.

Comment 11

[Navin Gupta]

Attachment: patch w/ GetSupportsThreading

Comment 12

[David :Bienvenu]

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

thx, great, sr=bienvenu

Comment 13

[Navin Gupta]

fix checked in.

Comment 14

[[e-mail] (gone - use [e-mail] instead)]

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.

Comment 15

[David :Bienvenu]

yes, lxr verification is fine.

Comment 16

[Gayatri Rimola]

I agree - thanks for verifying this!

Comment 17

[[e-mail] (gone - use [e-mail] instead)]

Verified FIXED.

All files were checked in on August 29th, 2002.