Bug 28196 - In page anchors on additem.pl don't always go to the right place
Summary: In page anchors on additem.pl don't always go to the right place
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Phil Ringnalda
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-04-22 16:45 UTC by Lucas Gass
Modified: 2023-12-28 20:47 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.02,22.11.08


Attachments
Bug 28196: make sure hash is always appended last (2.09 KB, patch)
2021-04-22 20:50 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 28196: make sure URL hash is appended last (1.87 KB, patch)
2021-06-23 22:06 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 28196: make sure URL hash is appended last (1.92 KB, patch)
2021-06-24 12:48 UTC, David Nind
Details | Diff | Splinter Review
Bug 28196: make sure URL hash is appended last (1.97 KB, patch)
2021-07-09 13:32 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28196: Insert searchid in URL correctly, so it doesn't follow the hash (1.67 KB, patch)
2023-06-20 02:01 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 28196: Insert searchid in URL correctly, so it doesn't follow the hash (1.72 KB, patch)
2023-06-26 14:03 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 28196: Insert searchid in URL correctly, so it doesn't follow the hash (1.77 KB, patch)
2023-07-10 20:45 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass 2021-04-22 16:45:19 UTC
To recreate:

-Do a search within the staff client
-Click on any results and go to the detail page
-Click the edit button on any item in that record, it should take you to a URL like this: '/cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=55&itemnumber=117#edititem&searchid=scs_1619109565224'


This URL has the '#edititem' anchor in it, but since it is in between 2 other params and the '&searchid=' param has been appended after the anchor it doesn't work. 

If you edit the URL manually to be this order it will work: 

/cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=55&itemnumber=117&searchid=scs_1619109565224#edititem
 

The anchor must go last. 

This is an annoyance to librarians, particularly for records with lots items attached to it.
Comment 1 Lucas Gass 2021-04-22 17:14:26 UTC
Some more info. about the hierarchical structure of URLs:

https://tools.ietf.org/html/rfc3986#section-3

The anchor being the least important needs to be last
Comment 2 Lucas Gass 2021-04-22 20:50:45 UTC
Created attachment 120027 [details] [review]
Bug 28196: make sure hash is always appended last

To test:

-Do a search within the staff client
-Click on any results and go to the detail page
-Click the edit button on any item in that record, you will be taken to the edit item screen but you will not be taken to the <h2 id="edititem"> placeholder
-Apply patch and try again
-This time when taken to the edit item screen you should be placed right at the <h2 id="edititem"> placeholder
Comment 3 Lucas Gass 2021-04-22 20:58:55 UTC
My patch fixes the problem by changing browser.js. Perhaps a better solution would be to add the searchid into the links inside each of the templates that call browser.js. It seems to only be called in a handful of templates:


root@kohadevbox:koha(master)$ git grep 'Asset.js("js/browser.js")'
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/ISBDdetail.tt:    [% Asset.js("js/browser.js") | $raw %]
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/MARCdetail.tt:    [% Asset.js("js/browser.js") | $raw %]
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/advsearch.tt:    [% Asset.js("js/browser.js") | $raw %]
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt:    [% Asset.js("js/browser.js") | $raw %]
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/labeledMARCdetail.tt:    [% Asset.js("js/browser.js") | $raw %]
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt:    [% Asset.js("js/browser.js") | $raw %]
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt:    [% Asset.js("js/browser.js") | $raw %]
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt:    [% Asset.js("js/browser.js") | $raw %]
koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt:[% Asset.js("js/browser.js") | $raw %]
koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt:[% Asset.js("js/browser.js") | $raw %]
koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-del.tt:    [% Asset.js("js/browser.js") | $raw %]
Comment 4 Phil Ringnalda 2021-04-23 19:24:32 UTC
Since we don't support Internet Explorer, why not use the URL object, and let it do the work of parsing for you?

Something like (untested, so I'll probably typo something)

let parsedurl = new URL($(this).attr('href'));
parsedurl.searchParams.set('searchid',me.searchid);
window.location = parsedurl.href;

Bonus advantage: if searchParams.set() sees that someone has screwed up and the URL already has two searchid params in it, it'll set one and delete the other, while doing it by hand would be adding a third.
Comment 5 Lucas Gass 2021-04-26 16:31:07 UTC
(In reply to Phil Ringnalda from comment #4)
> Since we don't support Internet Explorer, why not use the URL object, and
> let it do the work of parsing for you?
> 
> Something like (untested, so I'll probably typo something)
> 
> let parsedurl = new URL($(this).attr('href'));
> parsedurl.searchParams.set('searchid',me.searchid);
> window.location = parsedurl.href;
> 
> Bonus advantage: if searchParams.set() sees that someone has screwed up and
> the URL already has two searchid params in it, it'll set one and delete the
> other, while doing it by hand would be adding a third.

Thanks Phil,

As far as I understand it new URL() with a single argument must include an absolute path. You can give it a relative URL, but in that case you must always pass the base URL as the 2nd argument. 

If I'm wrong I am happy for someone to let me know of provide an alternate patch!
Comment 6 Phil Ringnalda 2021-04-27 03:59:41 UTC
Sorry, let parsedurl = new URL($(this).attr('href'),document.baseURI); - I didn't actually look at what $(this).attr('href') was.
Comment 7 Hakam Almotlak 2021-06-23 15:54:20 UTC
without the patch the problem exist when you try to search within the catalogues but it does not exist when you search for a catalogue then you try to edit an item from the same catalogue.
after the patch it does not fix the problem.
Comment 8 Lucas Gass 2021-06-23 22:06:38 UTC
Created attachment 122352 [details] [review]
Bug 28196: make sure URL hash is appended last

TO test:
1 Do a search within the staff client
2 Click on any results and go to the detail page
3 Click the edit button on any item in that record, it should take you to a URL like this: '/cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=55&itemnumber=117#edititem&searchid=scs_1619109565224'
4 Apply patch
5 Do steps 1 & 2 again
Click the edit button on any item in that record, it should take you to a URL like this: '/cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=55&itemnum    ber=117&searchid=scs_1619109565224#edititem'
6. If they URL hash is last the anchor should work and you'll be placed on the h2 element with and ID of edititem
Comment 9 David Nind 2021-06-24 12:48:13 UTC
Created attachment 122384 [details] [review]
Bug 28196: make sure URL hash is appended last

TO test:
1 Do a search within the staff client
2 Click on any results and go to the detail page
3 Click the edit button on any item in that record, it should take you to a URL like this: '/cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=55&itemnumber=117#edititem&searchid=scs_1619109565224'
4 Apply patch
5 Do steps 1 & 2 again
Click the edit button on any item in that record, it should take you to a URL like this: '/cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=55&itemnum    ber=117&searchid=scs_1619109565224#edititem'
6. If they URL hash is last the anchor should work and you'll be placed on the h2 element with and ID of edititem

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 Nick Clemens (kidclamp) 2021-07-09 13:32:52 UTC
Created attachment 122747 [details] [review]
Bug 28196: make sure URL hash is appended last

TO test:
1 Do a search within the staff client
2 Click on any results and go to the detail page
3 Click the edit button on any item in that record, it should take you to a URL like this: '/cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=55&itemnumber=117#edititem&searchid=scs_1619109565224'
4 Apply patch
5 Do steps 1 & 2 again
Click the edit button on any item in that record, it should take you to a URL like this: '/cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=55&itemnum    ber=117&searchid=scs_1619109565224#edititem'
6. If they URL hash is last the anchor should work and you'll be placed on the h2 element with and ID of edititem

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Nick Clemens (kidclamp) 2021-07-09 13:38:18 UTC
Oops, this breaks item editing, the form now has values like:
<input type="hidden" name="itemnumber" value="974searchid=scs_1625837723086">
Comment 12 Phil Ringnalda 2023-06-20 02:01:48 UTC
Created attachment 152478 [details] [review]
Bug 28196: Insert searchid in URL correctly, so it doesn't follow the hash

Test plan:
1. Search the catalog in the staff client for anything which will return
   at least two results
2. Click on any result to go to the bib detail page
3. Click the edit button for any item attached to the bib
4. Verify that the loaded URL contains &searchid=scs_(a number), and
   that the URL ends with #edititem
5. Make any change to the item, save, verify that your change was made
Comment 13 Phil Ringnalda 2023-06-20 02:03:40 UTC
One handy feature of using searchParams.set rather than concatenating strings to form a URL is that you don't have to remember to include the ampersand.
Comment 14 Sam Lau 2023-06-26 14:03:36 UTC
Created attachment 152686 [details] [review]
Bug 28196: Insert searchid in URL correctly, so it doesn't follow the hash

Test plan:
1. Search the catalog in the staff client for anything which will return
   at least two results
2. Click on any result to go to the bib detail page
3. Click the edit button for any item attached to the bib
4. Verify that the loaded URL contains &searchid=scs_(a number), and
   that the URL ends with #edititem
5. Make any change to the item, save, verify that your change was made

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 15 Katrin Fischer 2023-07-10 20:45:39 UTC
Created attachment 153295 [details] [review]
Bug 28196: Insert searchid in URL correctly, so it doesn't follow the hash

Test plan:
1. Search the catalog in the staff client for anything which will return
   at least two results
2. Click on any result to go to the bib detail page
3. Click the edit button for any item attached to the bib
4. Verify that the loaded URL contains &searchid=scs_(a number), and
   that the URL ends with #edititem
5. Make any change to the item, save, verify that your change was made

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Tomás Cohen Arazi 2023-07-12 17:31:40 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 17 Martin Renvoize 2023-07-18 10:08:13 UTC
Thanks for all the hard work!

Pushed to 23.05.x for the next release
Comment 18 Matt Blenkinsop 2023-07-18 14:23:41 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x