Bug 23601 - Middle clicking a title from search results creates two tabs or a new tab and a new window in Firefox
Summary: Middle clicking a title from search results creates two tabs or a new tab and...
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff Client (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement with 10 votes (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 25027
  Show dependency treegraph
 
Reported: 2019-09-12 21:42 UTC by Benjamin Daeuber
Modified: 2020-05-11 21:08 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
This fixes an issue in Firefox where middle-clicking or CTRL-clicking a title in the results screen of the staff client opens two new tabs.
Version(s) released in:
20.05.00, 19.11.06, 19.05.11


Attachments
Bug 23601: Prevent default for auxclick (1.59 KB, patch)
2020-03-30 15:19 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23601: Prevent default for auxclick (1.63 KB, patch)
2020-03-31 06:48 UTC, David Nind
Details | Diff | Splinter Review
Bug 23601: Prevent default for auxclick (1.71 KB, patch)
2020-03-31 14:23 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Daeuber 2019-09-12 21:42:20 UTC
When middle-clicking or ctrl-clicking a title in the results screen of the staff client, Firefox opens two new tabs.

To reproduce:

1. Use a relatively recent version of Firefox (I've tested this on 68esr and 69)
2. Search for an item in the staff interface
3. From the results screen, middle click or control click a title. 
4. Depending on your Firefox settings, Firefox will open an new window and a new tab or two new tabs. The popup blocker may also block the opening of these tabs.

This appears to be related to the preventdefault() being used here, which Firefox handles differently than other browsers.

There's the broader discussion of whether there's any need to use preventdefault() here at all. From the comments in the jquery it appears that someone thought it wasn't a good idea.
Comment 1 Owen Leonard 2019-09-17 12:33:27 UTC
I've noticed this before too, but have never taken a hard look at a solution. It has something to do with the click handler added by the "browse search results" buttons.
Comment 2 Katrin Fischer 2020-03-29 13:31:19 UTC
I just tested and things seem to work ok now - please reopen if you can still see the buggy behavior!
Comment 3 Nick Clemens 2020-03-30 15:06:07 UTC
Still happens for me
Comment 4 Nick Clemens 2020-03-30 15:19:49 UTC
Created attachment 102089 [details] [review]
Bug 23601: Prevent default for auxclick

The issue appears to be that the default action is not prevented for middle click because it registers
an 'auxclick' event as opposed to a 'click' event

To test:
1 - Perform a search in staff client
2 - Shift-click and hold on a result
3 - Note a new tab opens
4 - Release the click, no change
5 - Middle click and hold on a result
6 - New tab opens
7 - Release, a second new tab opens
8 - Apply patch
9 - Reload page
10 - Middle click and hold
11 - New tab opens
12 - Release
13 - No new tab
Comment 5 Katrin Fischer 2020-03-30 15:35:46 UTC
Thx Nick!
Comment 6 David Nind 2020-03-31 06:48:46 UTC
Created attachment 102102 [details] [review]
Bug 23601: Prevent default for auxclick

The issue appears to be that the default action is not prevented for middle click because it registers
an 'auxclick' event as opposed to a 'click' event

To test:
1 - Perform a search in staff client
2 - Shift-click and hold on a result
3 - Note a new tab opens
4 - Release the click, no change
5 - Middle click and hold on a result
6 - New tab opens
7 - Release, a second new tab opens
8 - Apply patch
9 - Reload page
10 - Middle click and hold
11 - New tab opens
12 - Release
13 - No new tab

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2020-03-31 06:50:40 UTC
Tested on Ubuntu 18.04 with Firefox 74.0 and Firefox Developer Edition 75.0b11 (64-bit editions).
Comment 8 Jonathan Druart 2020-03-31 12:54:44 UTC
I cannot recreate that (Firefox 68.4.1esr)
Comment 9 Nick Clemens 2020-03-31 13:33:15 UTC
(In reply to Jonathan Druart from comment #8)
> I cannot recreate that (Firefox 68.4.1esr)

I do on Firefox 74.0
Comment 10 Jonathan Druart 2020-03-31 14:18:25 UTC
(In reply to Nick Clemens from comment #9)
> (In reply to Jonathan Druart from comment #8)
> > I cannot recreate that (Firefox 68.4.1esr)
> 
> I do on Firefox 74.0

Wait, I actually do! Lucky me!
Comment 11 Jonathan Druart 2020-03-31 14:23:35 UTC
Created attachment 102141 [details] [review]
Bug 23601: Prevent default for auxclick

The issue appears to be that the default action is not prevented for middle click because it registers
an 'auxclick' event as opposed to a 'click' event

To test:
1 - Perform a search in staff client
2 - Shift-click and hold on a result
3 - Note a new tab opens
4 - Release the click, no change
5 - Middle click and hold on a result
6 - New tab opens
7 - Release, a second new tab opens
8 - Apply patch
9 - Reload page
10 - Middle click and hold
11 - New tab opens
12 - Release
13 - No new tab

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 12 Jonathan Druart 2020-03-31 14:44:43 UTC
Let's do fix that correctly, please?

See bug 25027.
Comment 13 Martin Renvoize 2020-04-06 11:22:40 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 14 Joy Nelson 2020-04-29 19:30:17 UTC
backported to 19.11.x for 19.11.06
Comment 15 Lucas Gass 2020-05-11 21:08:02 UTC
backported to 19.05.x for 19.05.11