In Nightly builds, possibly after bug 743198 landed, clicking on the "Full Screen" context <menuitem> in the attached testcase fails, giving the message: Request for fullscreen was denied because Element.requestFullscreen() was not called from inside a short running user-generated event handler. Was the onclick handler for <menuitem> usecase missed when the new Fullscreen API was implemented? The prefixed Fullscreen API works in Firefox 46 and earlier for this testcase.

Xidorn, wdyt?

No, it is not a regression from bug 743198, it is a regression from e10s. The testcase works perfectly fine if it is opened in a non-e10s window.

And this is about whether IsHandlingUserInput works for <menuitem> click, which could affect not only fullscreen, but also various other things including window.open and copy/cut.

Review commit: https://reviewboard.mozilla.org/r/42913/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42913/

Comment on attachment 8735697 [details] MozReview Request: Bug 1260102 - Pass isHandlingUserInput through process boundary for content menu command. r=mrbkap https://reviewboard.mozilla.org/r/42913/#review39503 I'm marking ship-it but it seems like the test could use some helpers more effectively. ::: dom/html/test/browser_content_contextmenu_userinput.js:35 (Diff revision 1) > +add_task(function* () { > + let tab = gBrowser.addTab(kPage); > + let browser = tab.linkedBrowser; > + gBrowser.selectedTab = tab; > + registerCleanupFunction(() => gBrowser.removeTab(tab)); > + yield waitForDocLoadComplete(); Any reason not to use BrowserTestUtils.withNewTab? ::: dom/html/test/browser_content_contextmenu_userinput.js:39 (Diff revision 1) > + registerCleanupFunction(() => gBrowser.removeTab(tab)); > + yield waitForDocLoadComplete(); > + > + gMessageManager = browser.messageManager; > + gMessageManager.loadFrameScript( > + `data:,(${frameScript.toString()})();`, false); Likewise, why not use ContentTask to avoid having to do your own serialization?

https://reviewboard.mozilla.org/r/42913/#review39503 > Any reason not to use BrowserTestUtils.withNewTab? Just because I was not aware of them :) > Likewise, why not use ContentTask to avoid having to do your own serialization? Same here. But actually these two helper functions do not affect the length of the test a lot in terms of line count. I'd really hope if there could be a waitForMessage helper.

Comment on attachment 8735697 [details] MozReview Request: Bug 1260102 - Pass isHandlingUserInput through process boundary for content menu command. r=mrbkap Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42913/diff/1-2/

From IRC: > mrbkap> xidorn: Hey, a couple of things: you probably want to yield ContentTask.spawn to give it a chance to run in the child. > mrbkap> xidorn: But also, you can do |let promise = ContentTask.spawn(browser, null, function* () { yield new Promise(resolve => { ...addEventListener(..., resolve); }); }); yield promise;| Add ni? myself so that I remember to push a followup later.

https://hg.mozilla.org/integration/mozilla-inbound/rev/0a0465d2c85b8b3bd6cc38dede241e839cfa7bc0 Bug 1260102 followup - Simplify newly added test with util functions.

Xidorn, should this fix be uplifted to Aurora 47 in preparation for our e10s experiments on Beta 47?

Comment on attachment 8735697 [details] MozReview Request: Bug 1260102 - Pass isHandlingUserInput through process boundary for content menu command. r=mrbkap Approval Request Comment [Feature/regressing bug #]: e10s [User impact if declined]: window.open/fullscreen/copy etc. may not work if they are invoked from context menu created from <menu>/<menuitem> element in the content [Describe test coverage new/current, TreeHerder]: new test added [Risks and why]: doesn't seem to be risky [String/UUID change made/needed]: n/a

Hi Xidorn, this is a pretty big change. Could it negatively affect the non-e10s behavior? Since 47 will ship with e10s off I need to make sure we don't regression quality on that front while taking new uplifts to enable Beta 47 e10s experiment.

This doesn't seem to be a big change. Not counting the test, it is just +24 -10 lines. I cannot predict, but I think it shouldn't affect non-e10s behavior, especially given that there is a new test is added.

Comment on attachment 8735697 [details] MozReview Request: Bug 1260102 - Pass isHandlingUserInput through process boundary for content menu command. r=mrbkap e10s related, Aurora47+

has problems/conflicts uplifting to aurora Tomcats-MacBook-Pro-2:mozilla-central Tomcat$ hg graft --edit -r 928fe146f1c1 grafting 337073:928fe146f1c1 "Bug 1260102 - Pass isHandlingUserInput through process boundary for content menu command. r=mrbkap" merging browser/base/content/content.js merging dom/html/test/browser.ini warning: conflicts while merging dom/html/test/browser.ini! (edit, then use 'hg resolve --mark')

https://hg.mozilla.org/releases/mozilla-aurora/rev/b11fafb3a4f3 https://hg.mozilla.org/releases/mozilla-aurora/rev/186ef42b33c1