browser_treeWidget_keyboard_interaction.js tests widget for devtools as web content. Therefore, if we'll stop dispatching keypress events for non-printable keys and key combinations, widget fails to handle them since they use keypress events for handling arrow keys etc. It's easy to fix this bug with changing only tree widget or making the test to chrome. However, in strictly speaking, we should stop using keypress events in chrome code too if it's easy to fix. So, I'll investigate if all widget in devtools can be rewritten with keydown event listeners.

FYI: I tried to change all keypress event listeners under devtools/client, but it caused a lot of oranges and I don't understand which keypress event listeners missed to catch expected events due to preventing default of preceding keydown event. (See comment 2's tryserver result.)

Sorry for the delay, I hope to review this by Monday at the latest.

Comment on attachment 8953004 [details] Bug 1439500 - Make devtools/client/shared/widgets handle non-printable keys and key combinations with keydown event https://reviewboard.mozilla.org/r/222258/#review228912 Thanks for working on this! :) These changes looks reasonable to me. ::: commit-message-994a6:1 (Diff revision 1) > +Bug 1439500 - Make devtools/client/sared/widgets handle non-printable keys and key combinations with keydown event r?jryans Nit: sared -> shared

Is it correct that all `keypress` listeners for non-printables in content documents must be converted to `keydown`? In addition, is it correct that for chrome documents, no change is required, but it is suggested to convert them as well as a best practice? Do we plan to leave chrome documents like this forever, or will there be an eventual migration there as well?

Thank you for your review! (In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #9) > Is it correct that all `keypress` listeners for non-printables in content > documents must be converted to `keydown`? Ideally, yes. However, if keydown event's preventDefault() is called, following keypress event won't be fired. So, if a keypress event listener handles a key before another one but the another one is just replaced with keydown event handler, the handling order is swapped. So, this point makes me difficult to rewrite a lot of chrome keypress event listeners with keydown event listeners. > In addition, is it correct that for chrome documents, no change is required, > but it is suggested to convert them as well as a best practice? So, yes. > Do we plan > to leave chrome documents like this forever, or will there be an eventual > migration there as well? Currently, I have no plan because this change even in chrome makes Thunderbird and Seamonkey broken completely. But when you and your colleagues start to handle keyboard events newly, I'd be happy if you use keydown event listener as far as possible.

Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/efa7b8392cac Make devtools/client/shared/widgets handle non-printable keys and key combinations with keydown event r=jryans

(Just for future investigation... I was confused why only this these few widgets were impacted. `browser_treeWidget_keyboard_interaction.js` creates a data: URI document inside the toolbox to use as the UI during testing, so it follows the new content document rules for these key events. In our production usage of the toolbox, panels load as chrome documents, so they aren't currently impacted, but it would be better to update them anyway.)