Definitely a bug. Thanks for reporting!

Looks like buttons are designed to be activated as spaces. 

We explicitly disable this for AppListItemView's [1] so I'll do the same for the chip views as well.


[1] https://cs.chromium.org/chromium/src/ash/app_list/views/app_list_item_view.cc?q=app_list_item_view.cc&sq=package:chromium&g=0&l=612

catching this event in suggestion_chip_view wasn't enough: it appears the button.cc code gets called twice, and only one of the paths was through suggestion_chip_view

((Newcomer and I talked about it, I gave him my findings and it does look like button is handling events twice. he can take it from here))

Something I considered while working on  Issue 899094 :

The way AppListItemView::OnKeyPressed ignores space has been superseded by virtual bool Button::IsTriggerableEvent(const ui::Event& event);

One way to fix this may be for *all* Button subclasses in the app list to inherit from an AppListButton class that overrides IsTriggerableEvent and ensures:
 - space goes to the text field (and doesn't activate buttons)
 - Ctrl+Space always goes to the ash accelerator handler in order to switch keyboard layouts

then individual button types within applist code probably won't need to override OnKeyPressed or View::SkipDefaultKeyEventProcessing() 

Thanks tapted, this is a pretty good solution and will prevent us from chasing these bugs in the future. I'm going to talk to UX and see if we want to also implement this behavior in the shelf. 

I think we need consistency between all app icons in CrOS.

Bulk moving <p-1's to the next milestone because we branched to M-73.

waiting for UX feedback.

After thinking on this, I think UX will keep "space to activate" because this is used everywhere in CrOS system ui. I believe A11y users use this as well.

But, shiba, do you think we should special case app icons and suggested chips, and ignore space to activate?