47636 - Can't go back to multi-engine search results (user feedback)

Status: VERIFIED FIXED

(SeaMonkey :: Search, defect, P2)

Description

[german]

Steps:
Set sidebar search to advanced, use multiple engines, run a search
Go to any page listed as a link in the results
Attempt to go back to search results summary page - Back is disabled
Users in usability testing strongly expected this to work and did not know how 
to get the results page back like they would with a single engine search
Recommend: 
users should be able to go back to search results page (as they do not 
understand and care for the fact that it is internally generate). Back is still 
one of the most often used means of navigating to a known place

Comment 1

[german]

nom. for nsbeta3

Comment 2

[Claudius Gayle]

nav triage team: rjc, we need info. We'd like to know how 'hard' this is in 
order to assess the risks for nsbeta3.

claudius: Radha, isn't there a similar bug about this same issue somewhere? 
(because SH doesn't know about the search results page)

Comment 3

[Radha on family leave (not reading bugmail)]

My guess is results are set in the content area with window.content.location OR 
window.content.href. Used to be that these urls would not get in to SH. But 
nisheeth fixed this on sunday night 8/7. So I would give this another try now.

Comment 4

[Robert John Churchill]

QA: please test.

Comment 5

[Claudius Gayle]

Do you want a new bug? You can now 'go back' to the results page but there are no results there. Reloading didn't help. So I get 
he results page - but no results. This is with the 2000080808 builds on RHLinux 6.1

Comment 6

[Robert John Churchill]

Hmmm... doesn't look like onload() handlers are being re-run when going back to 
a page.  Ben, can you test this?

Comment 7

[johng]

nav triage team:
nsbeta3+

Comment 8

[Ben Goodger (use ben at mozilla dot org for email)]

ok I have a fix for this, but it's slow, and a rather nasty hack. 

Problems:
 - chrome URLs are added to sh as file urls. thus you get 'access to xpconnect 
denied' errors when you try to go back to the results page. I hacked around this 
(prepare to cringe) by trying to load a dummy service, catching the js error if 
xpconnect was denied (which it was), and reloading the document as a chrome URL. 
I also introduced several other hacks to ensure that the query was remembered.

This is SLOW. very SLOW (not to mention dirty). The document is loaded twice, 
and the search has to be re-run. It would be nicer if a) chrome urls were stored 
in the sh as such, and b) if there was some way of caching the search results so 
that the tree could be built from memory/disk. 

rjc? radha? 

Comment 9

[Ben Goodger (use ben at mozilla dot org for email)]

fixed. 

Comment 10

[Radha on family leave (not reading bugmail)]

can you attach the patch for this fix?

Comment 11

[Claudius Gayle]

sorry for the delay... This doesn't work. It comes close though.

Do a multi-search, click a result, then follow the link that appears in the bottom frame.

Then Click back, I get the page before the search results page in SH. However if I click forward I now get the 
aggregate search results page. The page after the individual link I followed is lost to SH.

Good news:
We are now able to store and access the aggregate search results page in Session History.

Bad news:
It is not indexed correctly at all and leads to loss of othe SH items.

REOPENING, I gave this a go with the 2000082108 build on Win98

Comment 12

[Ben Goodger (use ben at mozilla dot org for email)]

radha -
I've checked in my dirty hack (dummy xpconnect test, reload as chrome if fail)

I'm not sure who is responsible for converting chrome URLs before they get 
stored in SH, but wherever that is done, it probably shouldn't happen, or if it 
needs to happen for some other reason, the chrome url should be maintained and 
pushed into SH. 

Comment 13

[Ben Goodger (use ben at mozilla dot org for email)]

cladius - 
the problem you describe is interesting. I tried it out and it appears that the 
multiple results page (a chrome url) never appears in SH. 

Here's what I see:
- perform the steps described (multiple engine search, click a result in the 
search results list once, then click the link to that result in the iframe 
below, then click back)
- I'm taken back two pages as claudius describes. If I click forward, I'm 
returned to the multiple engine results search page (chrome URL) again. In 
either case, the search results page does not appear in the back and forward 
button session history menus, which is kinda odd. 

radha - 
can you take a look at this? is there any reason this chrome url 
(chrome://communicator/content/search/internetresults.xul) would not be properly 
stored in SH? (so that it did not appear in the back/fwd button menus... but was 
in there such that I could navigate forward to it from a page back?)

Comment 14

[verah (gone)]

Nav triage team: Changing to P2. 

Comment 15

[Jim Roskind]

PDT agrees with P2.  This seems like it breaks advanced search functionality
froma user's perspective (when they expect "back" to work).

Comment 16

[Radha on family leave (not reading bugmail)]

Ben, I need some input here. After clicking on the result in the bottom frame, 
it appears that the link is redirected to the main content area. How do you do 
this? For some reason, when the url is redirected to the top frame, the load 
attribute, nsIChannel::LOAD_RETARGETED_DOCUMENT_URI is not set in the 
destination url's channel. So the result page is loaded with loadType 
loadHistory, which doesn't get entered in SH and there fore the mis-behavior in 
the back/forward buttons. Once I know that the above attribute is indeed set, I 
can see if the problem is in mscott's code which takes care of settimg proper 
loadType in docshell. Please update me, Thanks, Radha 

Comment 17

[Peter Trudelle]

PDT marking nsbeta3-

Comment 18

[Radha on family leave (not reading bugmail)]

Nominating for rtm. We know what the problem is. I'm waiting for a good patch
from mscott.

Comment 19

[Radha on family leave (not reading bugmail)]

Giving it to mscott, since the change has to happen in uriloader. Scott, this 
is that problem with urls redirected to "_top" don't have proper attributes set 
in nsIChannel so that it won't mess with the docshell's loadType. You gave me a 
patch, but it didn't quite work right. With little effort I think this can be 
fixed. 

Comment 20

[Radha on family leave (not reading bugmail)]

Removing NEED INFO. scott has all the info needed to fix this. Scott, the same 
problem appears in frames that retarget to _parent. So, _top and  _parent  need 
the same fix that targeted urls have.

Comment 21

[scottputterman]

adding rtm need info.

Comment 22

[Radha on family leave (not reading bugmail)]

Attachment: patch to uriloader.cpp and nsDSURIContentListener.cpp

Comment 23

[Scott MacGregor]

Unfortunately the call to Stop needs to be in there. Otherwise we have problems
when a url is redirected from say a mail window or some other window to a
browser window. The stop call kills any content currently being loaded into the
browser window in addition to clearing timers and things associated with the
current document it's showing.

If we comment that out, then we run into problems in the case were urls are
redirected on the fly to a browser window.

Comment 24

[Scott MacGregor]

Attachment: proposed fix (still needs more testing)

Comment 25

[Scott MacGregor]

This patch is working for me. Radha's going to help me test it some more.

Comment 26

[rpotts (gone)]

hey scott,

I'll (sr=rpotts) since this fixes the problem :-)  I think that in the long term 
(post 6.0) we need to reorganize how the docshell and the uriloader interact - 
something along the lines we talked about a couple of days ago...

-- rick

Comment 27

[Scott MacGregor]

changing rtm need info to rtm+ now that I have the reviews.
r=radha
sr=rpotts

Comment 28

[Phil Peterson]

rtm++. Back has to work

Comment 29

[Scott MacGregor]

fix checked into the tip and the branch.

Comment 30

[rods (gone)]

*** Bug 52898 has been marked as a duplicate of this bug. ***

Comment 31

[Claudius Gayle]

VERIFIED Fixed in Branch and Trunk builds 2000102008,2000101608