STR: 1. Open debugger 2. Notice how Ctr+Shift+P brings up the operators panel 3. Undock the window 4. Press Ctrl+Shift+P, nothing happens anymore Same goes for all the other keyboard shortcuts.

Keyboard shorctuts have changed. Ctrl+Shift+P is now Accel+P.

We have the same problem with the inspector (arrow keys in the markup panel). bug 818411

Victor, can you tell me if you see any other missing shortcuts?

(In reply to Paul Rouget [:paul] from comment #4) > Victor, can you tell me if you see any other missing shortcuts? All shortcuts don't work after docking/undocking.

webconsole.xul is probably affected too.

About comment 2: no, it's not related.

Actually, the shortcuts work if the focus is on the debugger toolbar buttons.

Summary of the situation: Each developer tool (WebConsole, Debugger, Inspector and StyleEditor) live in iframes. These 4 iframes are part of a same document: toolbox.xul. toolbox.xul lives in an iframe. This iframe is either located below the web page (so in browser.xul), or in a standalone window (toolbox-window.xul). To "undock" the tools, we move the toolbox.xul document from the browser.xul iframe into the iframe in toolbox-window.xul. To do so, we use nsIFrameLoaderOwner.swapFrameLoaders. See http://mxr.mozilla.org/mozilla-central/source/browser/devtools/framework/Toolbox.jsm#397 Problem: debugger.xul is composed of: - a XUL toolbar - an iframe (that hosts the code viewer, named SourceEditor) This bring us to: (browser.xul or toolbox-window.xul) > toolbox.xul > debugger.xul > dataURI - a set of <xul:keys> For example "Accel + P" brings a popup The bug is: after swapFrameLoaders is called (undocking the toolbox) if the focus is inside the dataURI iframe (code is focused) the <keys> don't work anymore If the toolbar (in debugger.xul) is focused (click on one of the button for example), the keys work.

I need some platform help here.

Hrm. Swapping the toolbox.xul frameloader between elements shouldn't affect the interaction of debugger.xul and its subframe, I wouldn't think... Neil, any idea what might be going on here with the <xul:keys>?

Hmm, does something odd happen with message managers.

(In reply to Boris Zbarsky from comment #11) > Hrm. Swapping the toolbox.xul frameloader between elements shouldn't affect > the interaction of debugger.xul and its subframe, I wouldn't think... > > Neil, any idea what might be going on here with the <xul:keys>? Not offhand; all that happens is that the document notices the keyset being added/removed from the document and asks the XBL service to add/remove the event handler appropriately. If there is a bug in the handling of keysets in swapped frames it could be verified by opening about:config in a tab, dragging it into another window and seeing whether Alt+Shift+R still works.

(In reply to neil@parkwaycc.co.uk from comment #13) > If there is a bug in the handling of keysets in swapped frames it could be > verified by opening about:config in a tab, dragging it into another window > and seeing whether Alt+Shift+R still works. about:config doesn't include any iframe afaik. I'm working on a test case to, at least, confirm the problem.

(In reply to Paul Rouget from comment #14) > (In reply to comment #13) > > If there is a bug in the handling of keysets in swapped frames it could be > > verified by opening about:config in a tab, dragging it into another window > > and seeing whether Alt+Shift+R still works. > > about:config doesn't include any iframe afaik. So the keys in toolbox.xul itself still work?

I've built a test case addon. Browser source code on github: https://github.com/paulrouget/bug814638-test-case STR: - installing the addon brings 2 windows (the second one is below the first one) - See windows "host1.xul" and "host2.xul" - host1.xul includes about:config - focus about:config (click on a pref), press "Accel + T", see the "42" alert. - the <key> is not in about:config, but in the parent window (debugger.xul), that also includes a textbox) - focus the textbox, press "Accel + T", see "42" again - click "swap" to swap the iframes - focus about:config, press "Accel + T", nothing happen - focus the textbox, press "Accel + T", see "42" So it appears that the key event is not crossing the inner iframe anymore. If we re-swap (back to the original configuration), the problem is still present. I hope that helps.

(In reply to neil@parkwaycc.co.uk from comment #15) > So the keys in toolbox.xul itself still work? Yes. As long as the focus is not in an inner iframe.

I don't know the history of swapFrameLoaders but I can imagine it was originally implemented for content frames and then tweaked to work with chrome frames. But chrome event handlers for chrome subframes don't need to be reset. Because swapFrameLoaders is resettting them, this means that keys in intermediate frames don't fire because the key event is bubbling up from the subframe to the parent frame without propagating through the swapped frame.

(In reply to neil@parkwaycc.co.uk from comment #18) > I don't know the history of swapFrameLoaders but I can imagine it was > originally implemented for content frames IIRC, yes > and then tweaked to work with > chrome frames. Was it ever really tested for chrome frames?

> I can imagine it was originally implemented for content frames and then tweaked to work > with chrome frames. Yes. It was originally done for tab D&D between windows, then we just loosened the restriction to allow it on chrome frames. Testing for chrome frames was pretty minimal, yeah. Neil, want to fix?

This might work, but I don't have time to (write a) test.

(In reply to neil@parkwaycc.co.uk from comment #21) > Created attachment 691942 [details] [diff] [review] > Draft patch > > This might work, but I don't have time to (write a) test. I can test your patch and write a test if needed.

Thank you Neil, your patch works. Can you take a look at the test? It works here if run alone: TEST_PATH=content/base/test/chrome/test_bug814638.xul make mochitest-chrome But, I have to add 3 files to the Makefile. Only one of them is a test. The 2 other files need to be in the makefile to be available via a chrome:// url. But then, they are run as test (and the mochitests get stuck).

Comment on attachment 693068 [details] [diff] [review] patch v1 >+ test_bug814638_host.xul \ >+ test_bug814638_frame.xul \ I think you have to call the extra files file_bug814638_whatever.xul

Who can review this patch?

Comment on attachment 693327 [details] [diff] [review] patch v1.2 r=me if: 1) A comment is added there saying that if ourType is chrome then the chrome event handler for subframes is already correct (and set to something in our document). 2) A comment is added in the ourType != otherType test that says that if we ever stop returning there we need to adjust this code accordingly.

Sigh. My intention was to push the patch and test with different metadata, but Mercurial defeated me. https://hg.mozilla.org/integration/mozilla-inbound/rev/8b6edf9f6b87 https://hg.mozilla.org/integration/mozilla-inbound/rev/d9546d42054c

Backed out for test_bug814638.xul timeouts: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=d9546d42054c https://hg.mozilla.org/integration/mozilla-inbound/rev/f1a10ed19a32

s/accel/control: https://tbpl.mozilla.org/?tree=Try&rev=af9fbf04835c