STR: - enter responsive mode - inspect an element - finish inspecting, click escape Expected: The inspector should close first Actual: Responsive mode exits
I guess the correct behavior is: 1) open tool A 2) open tool B 3) ESC -> close tool B 4> ESC -> close tool A The last tool should close first. I don't really want to have a "if inspector open first, then close" kind of thing. I'd prefer to have a more generic and natural mechanism. But doing that with just the DOM events is, I think, impossible. This is how it works today: Both the Inspector and the Responsive Mode attach a listener to the chrome window. When the user press escape, the first to be notified is the first that attached the listener. We can stop the propagation, but of course, this is useless because we use the same node. We can use preventDefault() and check .defaultPrevented, but the listener are called in this order: 1) attach A 2) attach B 3) call A 4) call B So we can't preventDefault(). To get the right order, one could listen during the bubbling, but then, we start checking if another tool is open, and I'd like to avoid that.
Comment on attachment 631364 [details] [diff] [review] v1 This approach works because the inspector listens to keypress during the bubbling, while the Responsive Mode does not.
When editing in the rule view with responsive mode enabled, Escape is closing responsive mode where it should be cancelling the edit.
The rule view/html panel both prevent propagation of the event after hitting escape, so we should cancel responsive mode in the bubble phase.
if the responsive mode catch to the key event in the bubble phase, we are not guaranteed that its listener will be called before the inspector listener. I'll find a better way to do that (like listening to a deeper node).
Comment on attachment 638641 [details] [diff] [review] v1.1 looks good, makes sense.
We'll need that in Aurora too.
Comment on attachment 638641 [details] [diff] [review] v1.1 [Approval Request Comment] Bug caused by (feature/regressing bug #): Responsive Design Tool: bug 749628 User impact if declined: hard to use the Responsive Tool with the Inspector (closing it doesn't do what's expected) Testing completed (on m-c, etc.): on m-c Risk to taking this patch (and alternatives if risky): not high (code is pretty simple) String or UUID changes made by this patch: none
Comment on attachment 638641 [details] [diff] [review] v1.1 [Triage Comment] Low risk, and we can always backout/disable in the coming weeks if need be. Approving for Aurora 15.
Verified fixed on FF 15b4 on Win 7, Ubuntu 12.04 and Mac OS X 10.6 using the STR in comment 1.