Esc key isn't closing HTML dialog elements

[shaunanoordin]

UX & Accessibility Overview

a.k.a. the Esc/Escape key isn't working for certain default web browser behaviour
Affects: anything that uses <dialog>; possibly any other standard HTML element/function that's supposed to react to the Esc key.

• Expected: 'Esc' key should close any open <dialog>
• Actual: 'Esc' is ignored.

The HTML dialog element can be used to open a popup or modal. This popup/modal should then close-able either by a custom close button that we program, or by pressing the 'Esc' key on the keyboard.

However, in PFE, this standard behaviour is disabled. In fact, it's possible that any standard browser behaviour that relies on the 'Esc' key is completely ignored.

Analysis

The problem was in our PJC's SugarClient, which has a copy of the primus library. Primus has a hack that specifically disables the 'Esc' key on the document level (document.onKeyDown)... which means the Esc key is disabled across the entirety of PFE.

Hack details

  //
  // Hack 2: If you press ESC in FireFox it will close all active connections.
  // Normally this makes sense, when your page is still loading. But versions
  // before FireFox 22 will close all connections including WebSocket connections
  // after page load. One way to prevent this is to do a `preventDefault()` and
  // cancel the operation before it bubbles up to the browsers default handler.
  // It needs to be added as `keydown` event, if it's added keyup it will not be
  // able to prevent the connection from being closed.
  //
  if (document.addEventListener) {
    document.addEventListener('keydown', function keydown(e) {
      if (e.keyCode !== 27 || !e.preventDefault) return;

      e.preventDefault();
    }, false);
  }

• as seen in PJC, prior to today's fix: https://github.com/zooniverse/panoptes-javascript-client/blob/e13d3059163ae623a9ec339bb53027169ade834f/lib/SugarClient/primus.js#L3789-L3804
• as seen in primus: https://github.com/primus/primus/blob/ed70022caa51da4752c0d4a13b6d3b79bb7a4a91/primus.js#L1159-L1174

Dev Notes

Protip: if you encounter situations where pressing a key doesn't trigger the expected behaviour, use Chrome's Inspect element -> Event Listeners panel. This will list listeners for the element you want to get working, plus anything that bubbles up to parent elements. You can delete handlers one by one, and by process of elimination, you can figure out what's been blocking your key presses.

Status

Solved (for now) by #6912 and zooniverse/panoptes-javascript-client#223

Credit to @eatyourgreens for solving the problem by removing the hack from PJC's copy of Primus, and opening an issue on Primus itself: primus/primus#782

Ideally, these hacks should be removed from Primus - otherwise, whenever PJC syncs it copy with the latest version of Primus, we'll run into the same problem all over again.

[eatyourgreens]

I've opened primus/primus#783 to remove that code from Primus itself. If that's accepted, then we shouldn't have to maintain our own edited copy of the client.

[eatyourgreens]

I think this might also be in Cam’s handover notes for Sugar, but the latest version of the Primus client is available from https://notifications.zooniverse.org/primus.js.

Whenever Sugar is updated and deployed with a new version of Primus, you should download the new client code and commit it to the Panoptes client. Then release a new version of the Panoptes client. For Sugar to work properly, the Primus server and Primus client should both be on the same version of Primus.