When doing a staff client catalog search with more than 1 page of results and clicking the last result on any search result page, the top-left navigation button for "next" is greyed out. The expected behaviour would be to move into the next page of results. The "next" navigation should only be prevented when the very last search result is selected in the detail-view. As suggested in the linked bug 12261, this is a regression from my attempt to fix the issue with the navigation letting the user moving out of bound in relation to the result array. As suggested in bug 12261, the original cause was in part due to the total number of results not being carried in the search cookie. Secondly, the offset value that was being used in the navigation checks shouldn't be used in there (the checks for allowing movement within the results) because the cookie value is only updated upon a result click and not when the user is navigating using the detail-view. I wrongly submitted a patch to bug 12261 since it was already pushed. The patch brings the total number of search results available (as I think it was orignally planned, since the non-existing value was being used) to the browser.js which handles the navigation and makes use of that. I marked this as blocking 12261 - not sure if this is right...
Created attachment 29210 [details] [review] Bug 12481: Staff client detail-view "next" link is greyed out when the last search result of any results page is clicked or navigated into When doing a staff client catalog search with more than 1 page of results and clicking the last result on any search result page, the top-left navigation button for "next" is greyed out. To test: 1) Do an intranet catalog search that has more than 1 page of results. 2) Click on the last result on a page that is not the last one & confirm that the navigation button "next" is greyed out. 4) Apply patch. 5) Do an intranet search with more than 1 but less results than fit on a single page. 6) Click on the last result on the page and confirm that the "Next" navigation button is greyed out. 6) Do an intranet search with more than 1 page of results. 7) Click on the last result of a page that's not the final page of the results & confirm that "Next" is not grey out. 8) Navigate to the last page of the results and click on the final result & confirm that "Next" is greyd out.
Does this patch needs signoff? If yes, you missed to change the status :) And if yes, I will change the status to failed QA, because the patch does not work if you get more than 1 page of results. me.curPos is the position of the result on the current page, not the position on the total number of results.
(In reply to Jonathan Druart from comment #2) > Does this patch needs signoff? > If yes, you missed to change the status :) > And if yes, I will change the status to failed QA, because the patch does > not work if you get more than 1 page of results. > me.curPos is the position of the result on the current page, not the > position on the total number of results. You're right, I'm obviously a bit confused with regards to the fundamental purpose of some of the js I'm looking at. However, think that making the total results available to the navigation js is a starting point.
Created attachment 29272 [details] [review] Bug 12481: Staff client detail-view "next" link is greyed out when the last search result of any results page is clicked or navigated into When doing a staff client catalog search with more than 1 page of results and clicking the last result on any search result page, the top-left navigation button for "next" is greyed out. Tested on newest Firefox & Chromium. Attempts to restore originally planned checks for navigation with the exception of not using results.length due to buggy behaviour where the results get concatenated upon page reload & "return to results" navigation - a separate issue? To test: 1) Do an intranet catalog search that has more than 1 page of results. 2) Click on the last result on a page that is not the last one & confirm that the navigation button "next" is greyed out. 4) Apply patch. 5) Do an intranet search with more than 1 but less results than fit on a single page. 6) Click on the last result on the page and confirm that the "Next" navigation button is greyed out. 6) Do an intranet search with more than 1 page of results. 7) Click on the last result of a page that's not the final page of the results & confirm that "Next" is not grey out. 8) Navigate to the last page of the results and click on the final result & confirm that "Next" is greyd out.
Newest patch does not apply - does not apply to master, attaching again.
Created attachment 29275 [details] [review] Staff client detail-view "next" link is greyed out when the last search result of any results page is clicked or navigated into When doing a staff client catalog search with more than 1 page of results and clicking the last result on any search result page, the top-left navigation button for "next" is greyed out. Tested on newest Firefox & Chromium. Attempts to restore originally planned checks for navigation with the exception of not using results.length due to buggy behaviour where the results get concatenated upon page reload & "return to results" navigation - a separate issue? To test: 1) Do an intranet catalog search that has more than 1 page of results. 2) Click on the last result on a page that is not the last one & confirm that the navigation button "next" is greyed out. 4) Apply patch. 5) Do an intranet search with more than 1 but less results than fit on a single page. 6) Click on the last result on the page and confirm that the "Next" navigation button is greyed out. 6) Do an intranet search with more than 1 page of results. 7) Click on the last result of a page that's not the final page of the results & confirm that "Next" is not grey out. 8) Navigate to the last page of the results and click on the final result & confirm that "Next" is greyd out.
Created attachment 29276 [details] [review] Bug 12481: Staff client detail-view "next" link is greyed out when the last search result of any results page is clicked or navigated into When doing a staff client catalog search with more than 1 page of results and clicking the last result on any search result page, the top-left navigation button for "next" is greyed out. Tested on newest Firefox & Chromium. Attempts to restore originally planned checks for navigation with the exception of not using results.length due to buggy behaviour where the results get concatenated upon page reload & "return to results" navigation - a separate issue? To test: 1) Do an intranet catalog search that has more than 1 page of results. 2) Click on the last result on a page that is not the last one & confirm that the navigation button "next" is greyed out. 4) Apply patch. 5) Do an intranet search with more than 1 but less results than fit on a single page. 6) Click on the last result on the page and confirm that the "Next" navigation button is greyed out. 6) Do an intranet search with more than 1 page of results. 7) Click on the last result of a page that's not the final page of the results & confirm that "Next" is not grey out. 8) Navigate to the last page of the results and click on the final result & confirm that "Next" is greyd out.
I suppose that the first patch is obsolete? Seems to be the same code. I applied the second patch, did a search wit a couple of pages as result, went to the first detail from one of the result pages and clicked "Next" until the end of this page. Then it seems that I am "trapped" on this result page, after the last entry the first entry of the same result page is displayed (should be the first entry of the next page).
(In reply to Marc Véron from comment #8) > I suppose that the first patch is obsolete? Seems to be the same code. > > I applied the second patch, did a search wit a couple of pages as result, > went to the first detail from one of the result pages and clicked "Next" > until the end of this page. Then it seems that I am "trapped" on this result > page, after the last entry the first entry of the same result page is > displayed (should be the first entry of the next page). This is not the only interesting behaviour I've come across with the navigation. There's another issue where the results of the page get concatenated infinitely on page reload, which originally led me to believe the results-array in the cookie was the whole results set. These should probably be reported separately? The behaviour you're describing should be unrelated to my patch and I've come across it before doing any changes. Can't be 100%, obviously. Related js (which I have not touched): var browseRecords = function (movement) { var newSearchPos = me.curPos + movement; if (newSearchPos > me.searchCookie.results.length - 1) { window.location = '/cgi-bin/koha/catalogue/search.pl?' + decodeURIComponent(me.searchCookie.query) + '&limit=' + decodeURIComponent(me.searchCookie.limit) + '&sort=' + me.searchCookie.sort + '&gotoPage=detail.pl&gotoNumber=first&searchid=' + me.searchid + '&offset=' + newSearchPos; } else if (newSearchPos < 0) { window.location = '/cgi-bin/koha/catalogue/search.pl?' + decodeURIComponent(me.searchCookie.query) + '&limit=' + decodeURIComponent(me.searchCookie.limit) + '&sort=' + me.searchCookie.sort + '&gotoPage=detail.pl&gotoNumber=last&searchid=' + me.searchid + '&offset=' + (me.offset - me.searchCookie.pagelen); } else { window.location = window.location.href.replace('biblionumber=' + biblionumber, 'biblionumber=' + me.searchCookie.results[newSearchPos]); } }
Comment on attachment 29275 [details] [review] Staff client detail-view "next" link is greyed out when the last search result of any results page is clicked or navigated into Obsoleting patch #1 (suppose it is replaced by patch #2)
Created attachment 29386 [details] [review] Bug 12481 - Follow up: Link to results is not refreshed when item is on next or prev result page Test plan for this follow up Reproduce issue without patch: Do a search that results in 3 or more pages. Print out list or do the same search in an other browser (to have a reference for the following) Go to a detail view and step through details to a detail that ist on result #3 Click on Link "Return to details" Result: The first page will display (instead of page #2) With patch, the correct page should display.
(In reply to Marc Véron from comment #11) > Created attachment 29386 [details] [review] [review] > Bug 12481 - Follow up: Link to results is not refreshed when item is on > next or prev result page > > Test plan for this follow up > > Reproduce issue without patch: > > Do a search that results in 3 or more pages. > Print out list or do the same search in an other browser (to have a > reference for the following) > Go to a detail view and step through details to a detail that ist on result > #3 > Click on Link "Return to details" > Result: The first page will display (instead of page #2) > > With patch, the correct page should display. Hi, I'm unable to reproduce what you're describing with only my patch applied. What I'm doing: 1. Do a search with 3 or more pages of results. 2. Select a result from the first page and navigate into the third page using the detail-view. 3. Select "return to results" on a results on the third page.
Comment on attachment 29386 [details] [review] Bug 12481 - Follow up: Link to results is not refreshed when item is on next or prev result page Hi Juhani, Strange, I tested again with your patch only and can not longer reproduce the behaviour. I used a result set with about 10 pages. Yesterday, I had two tabs open, in one I had the result list (>10 pages) and in the other, I stepped through the single views andgot the odd behaviour. I obsolete now my follow up patch and will sign-off your patch.
Created attachment 29403 [details] [review] Bug 12481: Staff client detail-view "next" link is greyed out when the last search result of any results page is clicked or navigated into When doing a staff client catalog search with more than 1 page of results and clicking the last result on any search result page, the top-left navigation button for "next" is greyed out. Tested on newest Firefox & Chromium. Attempts to restore originally planned checks for navigation with the exception of not using results.length due to buggy behaviour where the results get concatenated upon page reload & "return to results" navigation - a separate issue? To test: 1) Do an intranet catalog search that has more than 1 page of results. 2) Click on the last result on a page that is not the last one & confirm that the navigation button "next" is greyed out. 4) Apply patch. 5) Do an intranet search with more than 1 but less results than fit on a single page. 6) Click on the last result on the page and confirm that the "Next" navigation button is greyed out. 6) Do an intranet search with more than 1 page of results. 7) Click on the last result of a page that's not the final page of the results & confirm that "Next" is not grey out. 8) Navigate to the last page of the results and click on the final result & confirm that "Next" is greyd out. http://bugs.koha-community.org/show_bug.cgi?id=12481 This patch prevents the "Next" button on detail view to be grayed out at the end of e result page. Signed-off-by: Marc Véron <veron@veron.ch>
(In reply to Juhani Seppälä from comment #9) (...) > This is not the only interesting behaviour I've come across with the > navigation. There's another issue where the results of the page get > concatenated infinitely on page reload, which originally led me to believe > the results-array in the cookie was the whole results set. These should > probably be reported separately? I propose that you report this separately.
(In reply to Marc Véron from comment #15) > (In reply to Juhani Seppälä from comment #9) > > (...) > > This is not the only interesting behaviour I've come across with the > > navigation. There's another issue where the results of the page get > > concatenated infinitely on page reload, which originally led me to believe > > the results-array in the cookie was the whole results set. These should > > probably be reported separately? > > I propose that you report this separately. Thanks for looking into this Marc and helping with my regression. I should've been more precise there: it's the result array in the search cookie that's getting concatenated needlessly on page reloads - not sure if this can actually cause any harm currently, but the concatenation on page reloads doesn't look intentional in any case. I can create a new bug for this.
Created attachment 29408 [details] [review] Bug 12481: Staff client detail-view "next" link is greyed out when the last search result of any results page is clicked or navigated into When doing a staff client catalog search with more than 1 page of results and clicking the last result on any search result page, the top-left navigation button for "next" is greyed out. Tested on newest Firefox & Chromium. Attempts to restore originally planned checks for navigation with the exception of not using results.length due to buggy behaviour where the results get concatenated upon page reload & "return to results" navigation - a separate issue? To test: 1) Do an intranet catalog search that has more than 1 page of results. 2) Click on the last result on a page that is not the last one & confirm that the navigation button "next" is greyed out. 4) Apply patch. 5) Do an intranet search with more than 1 but less results than fit on a single page. 6) Click on the last result on the page and confirm that the "Next" navigation button is greyed out. 6) Do an intranet search with more than 1 page of results. 7) Click on the last result of a page that's not the final page of the results & confirm that "Next" is not grey out. 8) Navigate to the last page of the results and click on the final result & confirm that "Next" is greyd out. http://bugs.koha-community.org/show_bug.cgi?id=12481 This patch prevents the "Next" button on detail view to be grayed out at the end of e result page. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
(In reply to Juhani Seppälä from comment #16) > I can create a new bug for this. Did you create it?
(In reply to Jonathan Druart from comment #18) > (In reply to Juhani Seppälä from comment #16) > > I can create a new bug for this. > > Did you create it? Yes, was about to post it here for Marc as well: Bug 12510(In reply to Jonathan Druart from comment #18) > (In reply to Juhani Seppälä from comment #16) > > I can create a new bug for this. > > Did you create it? Yes, sorry, was about to post it here for Marc as well, see Bug 12510.
Pushed to master. Thanks Juhani!