Closed
Bug 150333
Opened 22 years ago
Closed 14 years ago
Next and Previous start value may need to be definable by search plugin.
Categories
(SeaMonkey :: Search, defect)
SeaMonkey
Search
Tracking
(Not tracked)
RESOLVED
INCOMPLETE
People
(Reporter: mmcguigan, Unassigned)
References
()
Details
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
application/zip
|
Details |
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.0) Gecko/20020605
BuildID: 2002060511
The start value used by nsinternetsearchservice is 0 and needs to be 1 in order
for results to display and increment properly with excite.
In
http://lxr.mozilla.org/seamonkey/source/xpfe/components/search/src/nsInternetSearchService.cpp
If I read the code correctly it is in this section of code on line 4622 that the
value in question is set to 0:
4621 // XXX get page
4622 PRInt32 errorCode, index = 0;
4623 PRInt32 factorInt = factor.ToInteger(&errorCode);
4624
4625 if (NS_SUCCEEDED(errorCode))
4626 {
4627 // if factor is garbled assume 10
4628 if (factorInt <= 0)
4629 factorInt = 10;
4630
4631 if (direction < 0)
4632 {
4633 // don't pass back a negative index!
4634 if (0 <= (page - 1))
4635 --page;
4636 }
4637 index = factorInt * page;
4638 }
4639
4640 return index;
4641 }
If the value for "index" could be defined in the plugin instead of the program
assuming 0 is the correct value, it should solve the problem.
Reproducible: Always
Steps to Reproduce:
1.Place this search plugin in your mozilla searchplugins directory.
# Start File Excite.src
# This is a beta search plugin and excite has been making changes to
# their search service every few days lately, so there is no gaurantee
# that this plugin will work at all in the near future.
<search
name="Excite"
action="http://msxml.excite.com/info.xcite/dog/webresults.htm"
method="GET"
>
<input name="qkw" user>
<input name="qcat" value="web">
<input name="qk" value="20">
<inputnext name="qi" factor="20">
<inputprev name="qi" factor="20">
<interpret
browserResultType = "result"
bannerStart="<table cellspacing=0 cellpadding=3 border=0 width=96%>"
bannerEnd="</table>"
resultListStart="</table><br>"
resultListEnd="<br> </font>"
resultItemStart=". "
resultItemEnd=" <br>"
>
<interpret
browserResultType = "result"
#
bannerStart="<!-- ----------Advertising.com Banner Code---------- -->"
#
bannerEnd="<!-- ----------Copyright 2000, Advertising.com---------- -->"
resultListStart="<tr> <td>"
resultListEnd="</td> </tr> </table>"
resultItemStart=". "
resultItemEnd=" </font><br>"
>
</search>
# end Excite.src
2. Perform search from sidebar or address bar using plugin.
Actual Results: The results start at and display as 0.-18.
Expected Results: The results should start at and display as 1.-20.
Reporter | ||
Comment 1•22 years ago
|
||
Please note that the search results display correctly if the following lines are
removed from the plugin:
<inputnext name="qi" factor="20">
<inputprev name="qi" factor="20">
However, removing those lines disables the next and previous buttons in the sidebar.
Summary: Next and Previous start value may need to be definable by search plugin. → Next and Previous start value may need to be definable by search plugin.
Updated•22 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 3•22 years ago
|
||
ok, the following URL, however, displays perfectly, giving results 1-20:
http://msxml.excite.com/info.xcite/dog/results?otmpl=dog/webresults.htm&qkw=test&qcat=web&qi=1&qk=20
This appears to be a classic "off by one" error, common where zero and one-index systems coexist. Excite's search result pages are 1-indexed (in actual fact, they display "qk" results from result "qi", inclusive, and don't seem to handle "qi=0").
What I would suggest is adding an "offset" property that simply offsets the factor. It should probably be called something else, though.
The "getIndex" function would set the index to index = (factor * page) + offset, with offset defaulting to zero. This avoids changing the behavior of existing plugins.
It might also be worth checking what the mac folks do in these cases, as I'm sure the sherlock "standard" was designed to take things like this into account.
It solves this particular case quickly and easily, and also allows for other search-engines that aren't zero-indexed in their additional results display. More complex search-engines (perhaps using letters to index results pages? who knows) can be dealt with when they become visible.
I'd offer to code this addition (searchservice.cpp seems well enough designed and commented to make an addition painless), but I'm unfamiliar with mozilla programming conventions, as well as how to actually create a patch.
Matthew, the "index" you observe being set to zero is only being set to that in preparation for "index = factorInt * page;" further down the function. This essentially gives you "currentresultspage * factor" which is correct in most cases, though in this case, one needs to be added for every page.
Another slight "bother" that I've noticed is that if the values in inputnext and inputprev have the same name, the value is placed in the URL twice. Is this broken? If so, I'll post a new bug for it.
Sam
Reporter | ||
Comment 4•22 years ago
|
||
Thanks sam,
On the issue of the value being placed in the URL twice. In testing, if the
"name" is the same for both one only needs to define inputnext. Inputprev will
assume the the same name and factor. From AltaVista.src for example:
<inputnext name="stq" factor="10">
<inputprev>
NOTE: The <inputprev> still needs to be there or the "Previous" button will be
disabled.
Using similar code in your Google plugin should remove the annoyance without any
adverse affects.
Unless Apple Sherlock handles this differently or we can find a test case that
fails we most likely will get a won't fix.
Joolz,
Does Apple handle this the same way?
If not and a bug doesn't already exist for this, we need to submit a bug.
Comment 5•22 years ago
|
||
> if the values in inputnext and inputprev have the same name, the value
> is placed in the URL twice. Is this broken? If so, I'll post a new bug
> for it.
See the bug 151390 (Google-URL duplicates substring "start=0"), is this the same
issue?
On a sidenote, Sam, is there a way to break your long lines? it's very hard to
read your bug reports if we have to scroll horizontally. Thanks.
Reporter | ||
Comment 6•22 years ago
|
||
Added Sam to CC
Yes that is the issue.
http://bugzilla.mozilla.org/show_bug.cgi?id=151390
Adding post to that discussion.
Comment 7•22 years ago
|
||
Erich,
Sorry about the long lines.
I think I'll post it as a bug in bugzilla. It seems that when posting
via IE6.0 sp1 the lines do not wrap in the textarea. Rather, they do
wrap in the composition textarea, but the resulting bugzilla post
doesn't seem to match.
Posts seem to work fine from mozilla, though.
The other bug about the substrings is the same issue I mentioned, yes.
Sam
Comment 8•22 years ago
|
||
As far as I can tell the previous/next functionality is a mozilla extension to
the sherlock specification. For instance I can find nothing about it in
<http://developer.apple.com/technotes/tn/tn1141.html> and there is no similar
feature in the Sherlock application for MacOSX.
Julius
Updated•22 years ago
|
Reporter | ||
Comment 9•22 years ago
|
||
Note that buy.com plugin is also affected by this bug though less severe.
Details: First request for next results will show the same results you where
looking at. On second attempt it will show the next results without skipped
results.
FYI: please look at http://bugzilla.mozilla.org/show_bug.cgi?id=174661 before
attempting to use the buy.com plugin with a nightly build.
Comment 10•21 years ago
|
||
*** Bug 207179 has been marked as a duplicate of this bug. ***
Comment 11•21 years ago
|
||
*** Bug 202462 has been marked as a duplicate of this bug. ***
Comment 13•20 years ago
|
||
This patch adds allows for an optional adjust attribute for inputnext and
inputprev. I named the attribute adjust which I believe is consistent with the
attribute factor in that they both describe the operation to be performed. If
the attribute does not exist or doesn't successfully convert to an integer it
defaults to 0 so the behavior is consistent with the current behavior. I have
included the patch for bug 150337 in this patch and can remove it if this is
preferable. Thanks.
Attachment #174575 -
Flags: superreview?(alecf)
Attachment #174575 -
Flags: review?(caillon)
Comment 14•20 years ago
|
||
The attached zip file has a search plugin I created for XUL Planet that uses
the adjust attribute. Just unzip and place the two files in your apps
searchplugins directory and with the patch applied it will adjust the URL param
for the next and previous results when using the Next and Previous buttons.
Comment 15•20 years ago
|
||
Comment on attachment 174575 [details] [diff] [review]
patch
I started to work on fixing advanced mode next / previous functionality so I am
clearing the reviews. I also noticed that with the removal of the page
decrement that the direction argument is no longer needed. Comments on this as
well as the attached patch in progress are appreciated as always.
Attachment #174575 -
Flags: superreview?(alecf)
Attachment #174575 -
Flags: review?(caillon)
Assignee | ||
Updated•16 years ago
|
Product: Core → SeaMonkey
Updated•16 years ago
|
Assignee: samir_bugzilla → nobody
QA Contact: claudius → search
Target Milestone: Future → ---
Comment 16•15 years ago
|
||
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
Status: NEW → UNCONFIRMED
Comment 17•15 years ago
|
||
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 18•14 years ago
|
||
Now using openSearch backend. Closing as OBSOLETE
You need to log in
before you can comment on or make changes to this bug.
Description
•