We have the following JS:

// Allows ESC key to kill the throbber.
if (esc_key) {
  document.onkeydown = function(evt) {
    evt = evt || window.event;
    var isEscape = false;
    if ("key" in evt) {
      isEscape = evt.key == "Escape";
    } else {
      isEscape = evt.keyCode == 27;
    }
    if (isEscape) {
      $('.page-load-progress-lock-screen').remove();
    }
  };
}

Problem is evt.key and evt.keyCode can unexpectedly create JS type coercion due to the == comparison operator. Seems quite obvious for evt.keyCode to only require an integer but it's far less obvious exactly what to do for evt.key since you can have Escape, Esc or possibly other things. This will need some research, but the idea is to switch to using the === operator.

CommentFileSizeAuthor
#7 interdiff-3-7.txt685 bytesanavarre
#7 2882856-7.patch821 bytesanavarre
#3 2882856.patch458 bytesanavarre

Comments

anavarre created an issue. See original summary.

anavarre’s picture

Issue summary: View changes
anavarre’s picture

Status: Active » Needs review
StatusFileSize
new458 bytes
anavarre’s picture

Issue summary: View changes
anavarre’s picture

For evt.key https://www.w3.org/TR/uievents-key/#keys-ui is interesting in that it looks to be offering Escape and only it.

"Escape" The Escape (Esc) key.

This key was originally used to initiate an escape sequence, but is now more generally used to exit or "escape" the current context, such as closing a dialog or exiting full screen mode.

Problem is https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key/Key_V... also states in the UI keys notes:

In Internet Explorer 9 and Firefox 36 and earlier, the Esc key returns "Esc" instead of "Escape".

So we need to account for either Escape or Esc.

Status: Needs review » Needs work

The last submitted patch, 3: 2882856.patch, failed testing.

anavarre’s picture

Status: Needs work » Needs review
StatusFileSize
new821 bytes
new685 bytes
dom.’s picture

Status: Needs review » Reviewed & tested by the community

Patch applies and work fine to me.
RTBC

  • anavarre committed 838a888 on 8.x-1.x
    Issue #2882856 by anavarre: Prevent unexpected JS type coercion
    
anavarre’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.