Opened 3 years ago

Last modified 2 years ago

#18879 new defect

Ctrl/Alt+Arrow not ignored in Dojo 1.9, and later

Reported by: Earl Hood Owned by:
Priority: undecided Milestone: 1.14
Component: Dijit Version: 1.11.2
Keywords: Cc:
Blocked By: Blocking:

Description

For the project I am working on, we are upgrading Dojo from 1.8.10 to a later release (currently trying 1.11.2).

One behavioral problem we have encountered is the interference with keyboard nagivation of dijit widgets and global key event handlers we use. In our application, we process Alt+Left-Arrow and Alt+Right-Arrow keyboard events to perform specific action. This seems to work fine in Dojo 1.8.x and earlier. In Dojo 1.9 and later, this fails when certain dijit widgets have current focus.

Diving into the Dojo source code, it appears the behavior change occurs due to the introduction of dijit/_KeyNavMixin. In 1.8, dijit/_KeyNavContainer has explicit checks for Ctrl and Alt, and if present in the event, no action is performed by the widget. In 1.9, and later, dijit/_KeyNavMixin is managing the events, but I see no check for Ctrl and Alt, so _onLeftArrow() and _onRighArrow() get triggered regardless of any key modifiers.

This change in behavior appears to be unintended. If an arrow key is pressed with Ctrl or Alt modifier, then dijit should ignore the event, as it did in 1.8.x and earlier.

Change History (6)

comment:1 Changed 3 years ago by bill

I can see how _KeyNavMixin ignoring Ctrl and Alt keystrokes is convenient for most widgets. OTOH the current functionality lets widgets do things like:

_onLeftArrow: function(evt) {
   if(evt.shift) { ... }
   else { ... }
}

Admittedly I don't know of any _KeyNavMixin subclasses that are doing that.

comment:2 in reply to:  1 Changed 3 years ago by Earl Hood

Replying to bill:

I can see how _KeyNavMixin ignoring Ctrl and Alt keystrokes is convenient for most widgets.

Unsure if I agree with the convenient argument. What benefit is being achieved over older releases? What has happened is that Dojo behavior has changed, and it is was not documented. It is clear that in v1.8.x and earlier, the intent was when Ctrl and Alt modifiers are part of the event, Dijit would ignore the arrow action.

I am not aware of a GUI-based practice that would treat Alt/Ctrl?+Arrow the same as Arrow (w/no modifiers). BTW, Alt+Arrow is commonly known to go back/forward thru history in web browsers, and not go back/forward thru browser controls.

OTOH the current functionality lets widgets do things like:

_onLeftArrow: function(evt) {
   if(evt.shift) { ... }
   else { ... }
}

Admittedly I don't know of any _KeyNavMixin subclasses that are doing that.

It seems unreasonable to require someone to have to customize every single dijit widget that subclasses _KeyNavMixin to restore previous behavior.

Does the Dojo Toolkit team the behavioral change as a bug? If not, what is recommended to those that require the pre-1.9 behavior?

Thanks.

comment:3 Changed 3 years ago by bill

Unsure if I agree with the convenient argument.

You clearly misread what I wrote.

I am not aware of a GUI-based practice that would treat Alt/Ctrl?+Arrow the same as Arrow (w/no modifiers).

Me neither. Not sure what your point is.

Does the Dojo Toolkit team the behavioral change as a bug? If not, what is recommended to those that require the pre-1.9 behavior?

_KeyNavMixin is new to 1.9, so it didn't "change behavior", but subclasses like dijit/Tree changed behavior, so you could consider that a bug.

Anyway, I don't care either way on this. If someone wants to modify _KeyNavMixin to ignore ctrl/alt/shift/meta events, I won't object. It technically breaks _KeyNavMixin backwards compatibility, but you can spin it as a bug fix.

Applications can also get the old behavior simply by setting up advice, something like:

aspect.around(_KeyNavMixin.prototype, "_onContainerKeydown", function(origFunc){
    return function(evt){
        if(!(evt.ctrlKey || evt.altKey || evt.metaKey || evt.shiftKey)){
            origFunc.apply(this, arguments);
        }
    };
});

comment:4 Changed 3 years ago by Earl Hood

Thanks @bill. The aspect-around method appears to work, but it does depend on how the application is loaded. I have to make sure it is executed before any dijit modules are loaded so the custom _onContainerKeydown is invoked.

BTW, we are unfortunately use pre-AMD style due to the age of the project, so the following is the pre-AMD code that does what @bill's AMD-style code does:

dojo.require("dojo.aspect");
dojo.require("dijit._KeyNavMixin");
dojo.aspect.around(dijit._KeyNavMixin.prototype,
  "_onContainerKeydown", function(origFunc){
    return function(evt){
      if (!(evt.ctrlKey || evt.altKey || evt.metaKey || evt.shiftKey)) {
        origFunc.apply(this, arguments);
      }
    };
  }
);

_KeyNavMixin is new to 1.9, so it didn't "change behavior", but subclasses like dijit/Tree changed behavior, so you could consider that a bug.

I think there is intent for _KeyNavMixin to ignore modifiers since _onContainerKeypress explicitly checks for them and immediately returns if a modifier is part of the event. _onContainerKeydown does not make such check, with the exception where there is no keycode function and SPACE is pressed.

The addition of _KeyNavMixin changed the behavior of all widgets that use it. We do not use dijit/Tree, but the behavioral effect of _KeyNavMixin affects other dijit widgets. I see this as a regression behavioral bug of all widgets that use _KeyNavMixin. If regression is a concern for the Dojo team, then either all widgets that use _KeyNavMixin need to be modified, or _KeyNavMixin itself is modified to retain pre-1.9 behavior.

If the Dojo team decides to not consider this a bug, it would help to document this behavior change somewhere for those moving from pre-1.9 to a 1.9, or later, release. In such documentation, the code fragment posted by @bill can be included for those that need to retain the pre-1.9 behavior.

comment:5 Changed 3 years ago by dylan

Milestone: tbd1.13

comment:6 Changed 2 years ago by dylan

Milestone: 1.131.14
Note: See TracTickets for help on using tickets.