149931 - search plugin (sherlock) results in skipped results on google

Status: RESOLVED EXPIRED

(SeaMonkey :: Search, defect)

Description

[Sam Schinke]

Google searches using google, and navigated using the new "next" and "previous"
buttons in the sidebar result in half of the search results being missed,
without setting google preferences.

To reproduce:
1) Open mozilla, open two tabs.
2) Clear all cookies (to remove any google prefs, alternatively, delete
google.com's cookie).
3) Select one tab, open the sidebar, enter a google search.
4) Press the next button in the sidebar.
5) Select the other tab, enter the same search. 
6) Press the next button at the bottom of google's search page.
7) Compare the displayed pages.

What occurs: The tab navigated using the sidebar displays results "21-30" (and
never displayed results 11-20), the tab navigated using google's website
displays results "11-20".

What should occur: Both pages should be identical, excepting minor differences
in the URL.

What is occuring is the "start" query argument is being incremented by 20 by the
sidebar, but google defaults to displaying 10 results per page.

The fix for this, however, is trivial (there are a few possible fixes). What is
needed is a way to force the sidebar queries to display a number of results
equal to the increment of the "start" argument. This will also prevent other
similar synchronization problems should someone set their preferences in google
to some unusual number of results per page (eg, prefs set to 50 per page, with
an increment of 20 would keep 30 results each time next is pressed). 

This can be done by modifying the plugin to the following (if "20" is the
desired default increment):

# Mozilla/Google plug-in by [e-mail]

<search 
   name="Google"
   description="Google Search"
   method="GET"
   action="http://www.google.com/search"
   update="http://www.google.com/mozilla/google.src"
   updateCheckDays=1   
>

<input name="q" user>
<input name="sourceid" value="mozilla-search">
<inputnext name="start" factor="20">
<inputprev name="start" factor="20">
<input name="num" value="20">

<interpret 
    browserResultType="result" 
    resultListStart="<!--a-->" 
    resultListEnd="<!--z-->" 
    resultItemStart="<!--m-->" 
    resultItemEnd="<!--n-->"
>
</search>

Here's a link to the plugin, in it's source location (I don't know enough to
give the proper format for a patch):
http://lxr.mozilla.org/mozilla1.0/source/xpfe/components/search/datasets/google.src#1

The difference is the "<input name="num" value="20">" line, which sends a
parameter telling google we want 20 results per page.

I will test this modification and post back. I'll also give the other default
plugins a look-see for similar problems.

Also, as an aside, the <inputnext> and <inputprev> tags are as-yet undocumented.

Perhaps the default search plugins should be added to whatever testing protocols
are used (either that, or get http://mycroft.mozdev.org to take over)?

Regards,
Sam

Comment 1

[Erich 'Ricky' Iseli]

Most probably a dupe of bug 149631.

*** This bug has been marked as a duplicate of 149631 ***

Comment 2

[Sam Schinke]

No, it isn't.

The issue in this bug is whether or not the behaviour specified by the "inputnext" and "inputprev" sherlock plugin tags match the default behaviour of the search-engine. If the search-engine only displays ten results per page, and the "factor" specified by inputnext/prev is twenty, half the results will be missed.

It has nothing to do with basic/advanced search, and will manifest in either.

There is also the issue of whether or not the behaviour of the search-engine can be changed via a cookie (like google can). In such cases you could get even more wildy erratic results display.

If you compare the proposed "google.src" file (sorry, I don't know exactly how to create a "patch") to the original, you will see that this is solved by adding another search parameter that specifies the number of results to display as identical to the factor.

I had also emailed the creator of the plugin at google ([e-mail]), and he replied that the changes looked good and that he wasn't aware of the <inputnext> and <inputprev> tags.

Anyways, if this explanation, as well as my original description of the bug doesn't convince you that this is a duplicate, feel free to re-mark it as a dupe, and I'll just forget about it.

Sam

Comment 3

[Erich 'Ricky' Iseli]

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

Comment 4

[Erich 'Ricky' Iseli]

Sam, sorry for the trouble. However your bug is very closely connected to bug
149631 due to the fact that if you set the values this way:
  <inputnext name="start" factor="10">
  <inputprev name="start" factor="10">
  <input name="num" value="10">
you'll still have a problem. Your hack (putting 20 instead of 10) is just a
hack, and not the solution to the problem.

Comment 5

[Sam Schinke]

The basic/advanced search problem seems to manifest regardless of the settings 
in the google.src file, yes. I wouldn't expect it to be any other way.

Can you describe the problem you were observing with 10 for all three values?

At the time, 20 for all the values worked fine for all the tests I performed.

I am now seeing slight problems in basic mode, with 10 for those values 
(namely, hitting previous seems to jump back by factor*2), but that doesn't 
make it the same bug as the other one.

Sam

Comment 6

[Sam Schinke]

This problem of moving double the factor when pressing the previous button 
should probably be listed as a seperate bug, but I believe I've found it's 
cause.

The "showMoreResults" function in search-panel.js:

http://lxr.mozilla.org/seamonkey/source/xpfe/components/search/resources/search-
panel.js#996

And the "computeIndex" function in nsInternetSearchService.cpp:
http://lxr.mozilla.org/seamonkey/source/xpfe/components/search/src/nsInternetSea
rchService.cpp#4482

Both decrement the "page" index by one when hitting "previous". "page" is then 
multiplied by "factor".

This side-effect of computeIndex is well-hidden by the fact that computeIndex 
is called via "getInputs" in InternetSearchService.cpp (line 4475). getInputs 
is called by "GetInternetSearchURL" in InternetSearchService.cpp, which is the 
function that is called from search-panel.js.

getInputs doesn't increment the page number when asked for the _next_ index, so 
it is somewhat inconsistent. Either the SearchService or the search-panel 
should be responsible for tracking the current index, not both.

Anyways, that's my interpretation of that oddity. It can be solved by removing 
lines 4659 to 4664 in nsInternetSearchService.cpp, but there might be other 
things that rely on this side-effect. It can also be solved by removing lines1 
011 and 1012 in search-panel.js, but that would probably be bad "practice" to 
rely on an undocumented side-effect of a function buried three calls deep.

Sam

Comment 7

[Matthew A. McGuigan]

I don't believe this bug is a duplicate of
http://bugzilla.mozilla.org/show_bug.cgi?id=149631.

Here is how I read this bug:

Mozilla-Search needs to read the factor value from the URL of the current page
before assuming where the factor starts and ends for the Next or Previous results.

Problem: If we ever get POST working, it won't work for sites that use POST,
unless someone knows a work around that I'm not aware of.

This bug is also affected or related to
http://bugzilla.mozilla.org/show_bug.cgi?id=150333.

FYI: A bug exists for the factor*2 problem. 
http://bugzilla.mozilla.org/show_bug.cgi?id=150337

Comment 8

[Sam Schinke]

I'll re-post my comments about the factor*2 bug there, thanks.

I think I can shed some light on your excite.com problem as well.

Comment 9

[Erich 'Ricky' Iseli]

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

Comment 10

[Robert Kaiser]

MASS-CHANGE:
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614

Comment 11

[Robert Kaiser]

MASS-CHANGE:
This bug report is registered in the SeaMonkey product, but still has no comment since the inception of the SeaMonkey project 5 years ago.

Because of this, we're resolving the bug as EXPIRED.

If you still can reproduce the bug on SeaMonkey 2 or otherwise think it's still valid, please REOPEN it and if it is a platform or toolkit issue, move it to the according component.

Query tag for this change: EXPIRED-20100420