Until now, the behavior was: Inspector open + ESCAPE pressed = Inspector closed Inspector open & Tilt open + ESCAPE pressed = only Tilt closed So currently, if Tilt is open and ESCAPE is pressed, we're not returning back to the Highlighter, but closing Inspector altogether. If this is the desired behavior, we can remove some redundant code from Tilt. However, I'm thinking it's not.
You're right, this is a bug. We should go back to the initial behavior.
I'll take a look at this.
Created attachment 590184 [details] [diff] [review] v1 The only change worth looking at is in browser/devtools/highlighter/inspector.jsm. I added a currentInstance getter in Tilt to get the visualizer for the current tab.
Created attachment 590188 [details] [diff] [review] v2 Using the bubbling trick. I left the currentInstance getter in there because it may be useful sometime!
Comment on attachment 590188 [details] [diff] [review] v2 This approach sounds good. Not sure this is needed: + e.preventDefault(); Why don't you just use `code = e.keyCode` ?
Created attachment 590193 [details] [diff] [review] v3
Comment on attachment 590193 [details] [diff] [review] v3 + canvas.addEventListener("keypress", this.onKeyPress, false); should this be , true? Do we want capturing? e.g., Inspector is now using: this.highlighter.highlighterContainer.addEventListener("keypress", this.onKeypress, true); (line 367, InspectorUI). and for completeness, you probably want to use preventDefault() as well as stopPropagation().
ok, scratch that bit about needing preventDefault() it's redundant.
(In reply to Rob Campbell [:rc] (robcee) from comment #8) > ok, scratch that bit about needing preventDefault() it's redundant. But I wanna!
Created attachment 592273 [details] [diff] [review] v4010