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)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: mmcguigan, Unassigned)

References

()

Details

Attachments

(2 files)

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=".&nbsp;&nbsp;" resultItemEnd="&nbsp;<br>" > <interpret browserResultType = "result" # bannerStart="<!-- ----------Advertising.com Banner Code---------- -->" # bannerEnd="<!-- ----------Copyright 2000, Advertising.com---------- -->" resultListStart="<tr> <td>" resultListEnd="</td> </tr> </table>" resultItemStart=".&nbsp;&nbsp;" resultItemEnd="&nbsp;</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.
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
helpwanted
Keywords: helpwanted
Target Milestone: --- → Future
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
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.
> 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.
Added Sam to CC Yes that is the issue. http://bugzilla.mozilla.org/show_bug.cgi?id=151390 Adding post to that discussion.
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
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
Depends on: 172120
Blocks: 172120
No longer depends on: 172120
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.
*** Bug 207179 has been marked as a duplicate of this bug. ***
*** Bug 202462 has been marked as a duplicate of this bug. ***
Added top100 keyword - evangelism triage for Moz 1.7
Keywords: top100
Attached patch patch (deleted) — Splinter Review
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)
Attached file testcase for patch (deleted) —
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 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)
Product: Core → SeaMonkey
Assignee: samir_bugzilla → nobody
QA Contact: claudius → search
Target Milestone: Future → ---
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
Status: UNCONFIRMED → NEW
Ever confirmed: true
Now using openSearch backend. Closing as OBSOLETE
Status: NEW → RESOLVED
Closed: 14 years ago
Keywords: helpwanted, top100
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: