Angular.js: Can't write space chars in input when it in tag with ng-click with ngAria since 1.7.3

[Sergi0Limit]

I'm submitting a ...

• [x] bug report
• [ ] feature request
• [ ] other

Current behavior:

When input/textarea in the wrapped tag with ng-click i can't wrote space chars in the text.

Expected / new behavior:

When the input / textarea in the wrapped tag with ng-click i should be able to write space characters in the text.

Minimal reproduction of the problem with instructions:
Demo https://codepen.io/sergi0limit/pen/mjYLGE

AngularJS version: 1.7.3

Browser: all

Anything else:

commit https://github.com/angular/angular.js/commit/6c224a2a6059d4a089ef396c880c4a6369f0be59

[hoeni]

We have the same problem here. When upgrading from 1.7.2 to 1.7.3 all presses of the space key are lost in all input and textarea fields.

Maybe there is something wrong with the following change (We do use ngAria, indeed)?:

ngAria: do not scroll when pressing spacebar on custom buttons (3a517c, #14665, #16604)

[Sergi0Limit]

@hoeni problem in ngClick that in ngAria.js. If you are using ngMaterial module than need ngAria module.

[petebacondarwin]

I think you are right @hoeni - this is because of that ngAria change. You can see in this version of the CodePen that pressing space causes the click handler: https://codepen.io/petebacondarwin/pen/OweOYK?editors=1111

I guess we need to ensure that ngAria does not prevent default on input/textarea elements?

[gkalpak]

Yeah, ngAria assumes that any element with ngClick is a custom button (adding the button role, handling ENTER/SPACEBAR as clicks, etc.).

Doing this for input/select/textarea elements (except for input[type"button/reset/submit"]) doesn't make sense indeed.

[Sergi0Limit]

@gkalpak but it's not realy true for all solutions. In my case i have multiple div as row in list and it have some fields and input field.
These divs can selected by click.

[gkalpak]

Not sure what you mean, @Sergi0Limit :confused:

[Sergi0Limit]

I mean... Blocking space chars in my case is this a bug or a feature?

[gkalpak]

Preventing tge default browser behavior (i.e. scrolling down) on the divs sounds like the right thing to do, no?

[Sergi0Limit]

Focus in the input, not on div. Why it do click instead of space?

[Narretz]

Here are my thoughts on this:
This is expected behavior.
ngAria automatically adds role=button to elements with ngClick that are not natively interactive. This is correct and necessary.

However, if you have an input or a button, and on one of its parent elements use ngClick, you don't want this ngClick to be role=button. This is because it's not logical to have two nested buttons. I'd assume screen readers and other assistive technology will have problems with this.

HTML actually disallows the nesting of most interactive elements. Of course, a div with a click event is not native interactive content, but once you include ngAria into the mix, you make it "interactive":
Here's a good summary for this: http://adrianroselli.com/2016/12/be-wary-of-nesting-roles.html

So I can think of two ways to go from here:

• use ng-aria-disable on the ngClick element
• use ng-event-click ng-on-click instead of ngClick on the element
• use addEventListener directly from a directive / service

[hoeni]

In our case, we have a rather complex form generator. If you click a whole form line with SHIFT and ALT pressed, it will open a form inspector (for form debugging by developers). This is the surrounding ng-click. I can confirm that removing the ngClick handler makes the space key work again in contained inputs.

On the one hand I can understand that ngAria should clearly regard space key press as a button press and suppress page scrolling when activated. On the other hand I see this as a rather breaking change I would not expect in a minor AngularJS Update without prior heads up.

@Narretz :

• ng-aria-disable on our ng-click element fixes our problem. Spaces work and also the click handler.
• ng-event-click instead of ngClick does not trigger the handler. I have never seen that notation, can you give me some pointer?

[Narretz]

Sorry, ng-event-click is wrong, correct is ng-on-click. https://codepen.io/narretz/pen/ajeZmZ This is also new in 1.7.3: https://docs.angularjs.org/api/ng/directive/ngOn

It's imo still debatable if this is a breaking change. Nesting interactive elements like this was wrong even before this change. Although we'll probably have to revert this if it turns out many apps are affected.

[Sergi0Limit]

@hoeni
Problem solved by ng-aria-disable me too.

[Sergi0Limit]

@Narretz
ng-on-click this is good idea) Thanks.

[petebacondarwin]

I think that ng-aria-disable is the best workaround. Thanks for the write up @Narretz. We should document that. (And perhaps the Material project should do so too! @Splaktar?)

[gkalpak]

Hm...I didn't realize this also affects child elements. In that case, I don't think this is a BC worth having.
Maybe we could change preventDefault() to only happen if the ngClick element is the target of the keydown event only (here):

 if (keyCode === 13 || keyCode === 32)
-  event.preventDefault();
+  if (event.target === elem[0]) event.preventDefault();
   scope.$apply(callback);
 }

We could additionally check whether nodeBlackList.includes(event.target.nodeName).

[Narretz]

Hm, I don't think labelling this as a BC is correct if we assume that setting ngClick -> input element is incorrect in the first place (with ngAria)
Even if we try to improve the space press behavior, the constellation still introduces a case where you have an element with role="button" and as a child another interactive element, and this incorrect based on aria rules: https://w3c.github.io/html-aria/#allowed-aria-roles-states-and-properties

I feel like we shouldn't make it easier again to introduce a suboptimal aria state in apps, especially with ngAria included.

[gkalpak]

Still, having people's apps break like this (in a patch release), is bad imo.
FWIW, there are apps that are not great ARIA-wise, but work. With this change they will break :confused:

[Splaktar]

I tested all of the AngularJS Material docs with 1.7.3 and I don't see any breakages there related to the space key being blocked on any kind of inputs. This is likely due to us not nesting inputs inside of interactive elements.

That said, I'm sure that many developers who are using AngularJS Material are doing this. I've seen it in the past (and warned people about it).

AngularJS Material recently fixed a related issue with ngAria where it was adding a role="button" to placeholders in our inputs. We were able to fix this issue via aria-hidden="true" but I don't believe that is necessarily the right fix for this issue.

I would suggest that the release notes should warn developers about this breaking change coming in the future, provide them with clear guidance on why nesting interactive elements is bad, and then move this change to a future release.

[mgol]

I agree with @gkalpak & @Splaktar we should fix this issue as it may break many users. We're (almost?) feature-frozen now so we should never re-apply this back, though. It's IMO too late for such disruptive changes, especially in a patch release, even if they're technically right.

[Narretz]

We can add an addtional check as suggested by @gkalpak:

check whether nodeBlackList.includes(event.target.nodeName).

and also possibly check if the target element has aria-roles that make it interactive (i.e. button).

This way, we get the better aria support and still allow this use case.