Hi Nika! Sorry to bother you again with this but I have a new test failure on the latest central: https://treeherder.mozilla.org/#/jobs?repo=try&revision=978c102bd686164ebe8c524ed7703734335c5e3a&selectedJob=284936975 The failing test is [test_bug814638.xhtml](https://searchfox.org/mozilla-central/source/dom/base/test/chrome/test_bug814638.xhtml) , which uses swapFrameLoader with iframes. Basically the test loads the following structure twice, in 2 different windows ``` <iframe> <!-- toplevel iframe --> #document <keyset><key key="T"/></keyset> <iframe src="about:mozilla">...</iframe> <!-- inner iframe --> </iframe> ``` The test calls swapFrameLoaders on the two top-level iframes, and after that triggers an event in one of the the "inner iframe" and checks that the <key> still executes the expected command. In order to work without the workaround, those iframes should be switched to type=content (or even <browser type=content>). But I don't think <keyset> works in content frames, and overall I don't know if the feature tested here is still relevant. It was added in https://bugzilla.mozilla.org/show_bug.cgi?id=814638 , seems like it was mostly added for devtools. And when we switched to using frames with type=content in devtools, part of the change was actually to reattach chromeEventHandler events after a call to swapFrameLoaders. So I wonder if we could just remove the test? But I also see that the code added in the bug is not directly attached to the workaround: https://searchfox.org/mozilla-central/rev/9b4b41b95cbcda63f565bdc24411e15248f91d83/dom/base/nsFrameLoader.cpp#741-744 . Should we also delete some of this code? Let me know what you think! (note: I don't know _why_ this test didn't fail the first time we tried to land this bug, at that time it was using XUL files instead of XHTML, but I'm not sure what this implies)
Bug 1585747 Comment 17 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
Hi Nika! Sorry to bother you again with this but I have a new test failure on the latest central: https://treeherder.mozilla.org/#/jobs?repo=try&revision=978c102bd686164ebe8c524ed7703734335c5e3a&selectedJob=284936975 The failing test is [test_bug814638.xhtml](https://searchfox.org/mozilla-central/source/dom/base/test/chrome/test_bug814638.xhtml) , which uses swapFrameLoader with iframes. Basically the test loads the following structure twice, in 2 different windows ```html <iframe> <!-- toplevel iframe --> #document <keyset><key key="T"/></keyset> <iframe src="about:mozilla">...</iframe> <!-- inner iframe --> </iframe> ``` The test calls swapFrameLoaders on the two top-level iframes, and after that triggers an event in one of the the "inner iframe" and checks that the <key> still executes the expected command. In order to work without the workaround, those iframes should be switched to type=content (or even <browser type=content>). But I don't think <keyset> works in content frames, and overall I don't know if the feature tested here is still relevant. It was added in https://bugzilla.mozilla.org/show_bug.cgi?id=814638 , seems like it was mostly added for devtools. And when we switched to using frames with type=content in devtools, part of the change was actually to reattach chromeEventHandler events after a call to swapFrameLoaders. So I wonder if we could just remove the test? But I also see that the code added in the bug is not directly attached to the workaround: https://searchfox.org/mozilla-central/rev/9b4b41b95cbcda63f565bdc24411e15248f91d83/dom/base/nsFrameLoader.cpp#741-744 . Should we also delete some of this code? Let me know what you think! (note: I don't know _why_ this test didn't fail the first time we tried to land this bug, at that time it was using XUL files instead of XHTML, but I'm not sure what this implies)
Hi Nika! Sorry to bother you again with this but I have a new test failure on the latest central: https://treeherder.mozilla.org/#/jobs?repo=try&revision=978c102bd686164ebe8c524ed7703734335c5e3a&selectedJob=284936975 The failing test is [test_bug814638.xhtml](https://searchfox.org/mozilla-central/source/dom/base/test/chrome/test_bug814638.xhtml) , which uses swapFrameLoader with iframes. Basically the test loads the following structure twice, in 2 different windows ```html <iframe> <!-- toplevel iframe --> #document <keyset><key key="T"/></keyset> <iframe src="about:mozilla">...</iframe> <!-- inner iframe --> </iframe> ``` The test calls swapFrameLoaders on the two top-level iframes, then triggers an event in one of the "inner iframes" and checks that the <key> still executes the expected command. In order to work without the workaround, those iframes should be switched to type=content (or even <browser type=content>). But I don't think <keyset> works in content frames, and overall I don't know if the feature tested here is still relevant. It was added in Bug 814638 , seems like it was mostly added for devtools. And when we switched to using frames with type=content in devtools, part of the change was actually to reattach chromeEventHandler events after a call to swapFrameLoaders. So I wonder if we could just remove the test? But I also see that the code added in the bug is not directly attached to the workaround: https://searchfox.org/mozilla-central/rev/9b4b41b95cbcda63f565bdc24411e15248f91d83/dom/base/nsFrameLoader.cpp#741-744 . Should we also delete some of this code? Let me know what you think! (note: I don't know _why_ this test didn't fail the first time we tried to land this bug, at that time it was using XUL files instead of XHTML, but I'm not sure what this implies)
Hi Nika! Sorry to bother you again with this but I have a new test failure on the latest central: https://treeherder.mozilla.org/#/jobs?repo=try&revision=978c102bd686164ebe8c524ed7703734335c5e3a&selectedJob=284936975 The failing test is [test_bug814638.xhtml](https://searchfox.org/mozilla-central/source/dom/base/test/chrome/test_bug814638.xhtml) , which uses swapFrameLoader with iframes. Basically the test loads the following structure twice, in 2 different windows ```html <iframe> <!-- toplevel iframe --> #document <keyset><key key="T"/></keyset> <iframe src="about:mozilla">...</iframe> <!-- inner iframe --> </iframe> ``` The test calls swapFrameLoaders on the two top-level iframes, then triggers an event in one of the "inner iframes" and checks that the <key> still executes the expected command. In order to work without the workaround, those iframes should be switched to type=content (or even <browser type=content>). But I don't think <keyset> works in content frames, and overall I don't know if the feature tested here is still relevant. It was added in Bug 814638 , seems like it was mostly added for devtools. And when we switched to using frames with type=content in devtools, part of the change was actually to reattach chromeEventHandler events after a call to swapFrameLoaders. So I wonder if we could just remove the test? But I also see that the code added in the bug is not directly attached to the workaround: [nsFrameLoader.cpp#741-744](https://searchfox.org/mozilla-central/rev/9b4b41b95cbcda63f565bdc24411e15248f91d83/dom/base/nsFrameLoader.cpp#741-744) . Should we also delete some of this code? Let me know what you think! (note: I don't know _why_ this test didn't fail the first time we tried to land this bug, at that time it was using XUL files instead of XHTML, but I'm not sure what this implies)