Enable containers for use in Nightly only starting in Firefox 50 (i.e. #ifdef NIGHTLY). Marking this as dependent on a list of implementation bugs. We may decide that a few of those (ex: favicon loads) don't actually block enabling on nightly.

Pushed by tvyas@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/19ac9881aa5e Enable experimental containers feature for Nighlty to test the OriginAttributes platform work and get some validation on the idea. r=past

Backed out for bc7 failures on OSX and Windows in browser_contextmenu.js: https://hg.mozilla.org/integration/mozilla-inbound/rev/79747869072b3b72c0484e79238d05bfb8efb1ac Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=19ac9881aa5eff2be6dbd50723fc36a110222715 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=30116152&repo=mozilla-inbound 07:08:40 INFO - 445 INFO TEST-PASS | browser/base/content/test/general/browser_contextmenu.js | checking item #0 (context-openlinkintab) name - 07:08:40 INFO - 446 INFO TEST-PASS | browser/base/content/test/general/browser_contextmenu.js | checking item #0 (context-openlinkintab) enabled state - 07:08:40 INFO - 447 INFO TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_contextmenu.js | checking item #1 (context-openlink) name - Got context-openlinkinusercontext-menu, expected context-openlink Looks like the test needs just an update.

Comment on attachment 8762718 [details] [diff] [review] Bug1276412-test.patch Review of attachment 8762718 [details] [diff] [review]: ----------------------------------------------------------------- I think this fix is ok. I'm not a reviewer for this component but I take the responsibility to give a r+.

Still one failure with this patch: 2100 INFO TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_contextmenu.js | menuitem context-copy has same accesskey as context-openlinkinusercontext-menu - Stack trace: chrome://mochitests/content/browser/browser/base/content/test/general/contextmenu_common.js:getVisibleMenuItems:70 chrome://mochitests/content/browser/browser/base/content/test/general/contextmenu_common.js:checkMenu:187 chrome://mochitests/content/browser/browser/base/content/test/general/contextmenu_common.js:checkContextMenu:133 chrome://mochitests/content/browser/browser/base/content/test/general/contextmenu_common.js:test_contextmenu:305

https://dxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/browser.dtd#288 <!ENTITY copyCmd.accesskey "C"> https://dxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/browser.dtd#460 <!ENTITY openLinkCmdInContainerTab.accesskey "C">

Changed the access key to "z", since most letters were taken. Gijs, can you take a look at this patch? I'm not sure why I need to put the blank line after the menu item, but without it, the patch fails. Maybe it is because the Open Link In Container Tab has a submenu. I need to land this as soon as possible, so if you could review sooner than later, that would be very appreciated! Thank you! https://treeherder.mozilla.org/#/jobs?repo=try&revision=3a82fa07a9ff

Added some comments.

Comment on attachment 8762754 [details] [diff] [review] Bug1276412-test-06-14-16C.patch Review of attachment 8762754 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me.

Pushed by tvyas@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f9c488fb21ff Fix context menu test that doesn't account for New Container Tab option. r=past https://hg.mozilla.org/integration/mozilla-inbound/rev/cce3120ab50c Enable experimental containers feature for Nighlty to test the OriginAttributes platform work and get some validation on the idea. r=past

Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/96d250dc0d85 Fix a typo in a comment, r=me

Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/027adaea2de2 Fix a typo in a comment - part 2, r=me

https://hg.mozilla.org/mozilla-central/rev/f9c488fb21ff https://hg.mozilla.org/mozilla-central/rev/cce3120ab50c https://hg.mozilla.org/mozilla-central/rev/96d250dc0d85 https://hg.mozilla.org/mozilla-central/rev/027adaea2de2

The text fix makes en-US use an external accesskey (a character that is not available in the original string), so the label will be displayed as "Open Link in New Container Tab (z)". No other characters available? I don't see any explanation for that in the bug.

(In reply to Francesco Lodolo [:flod] from comment #17) > The text fix makes en-US use an external accesskey (a character that is not > available in the original string), so the label will be displayed as "Open > Link in New Container Tab (z)". > > No other characters available? I don't see any explanation for that in the > bug. So many of the access keys were already taken; every letter I thought of. So I went with z, but we can easily change this to another one if you have a suggestion. If this is something we need to change sooner than later, please let me know and I will have someone look into it.

(In reply to Tanvi Vyas - behind on reviews [:tanvi] from comment #18) > So many of the access keys were already taken; every letter I thought of. > So I went with z, but we can easily change this to another one if you have a > suggestion. If this is something we need to change sooner than later, > please let me know and I will have someone look into it. Not really my call, but I think it's good to fix it, especially if you plan to enable the feature by default at some point.

(In reply to Francesco Lodolo [:flod] from comment #19) > (In reply to Tanvi Vyas - behind on reviews [:tanvi] from comment #18) > > So many of the access keys were already taken; every letter I thought of. > > So I went with z, but we can easily change this to another one if you have a > > suggestion. If this is something we need to change sooner than later, > > please let me know and I will have someone look into it. > > Not really my call, but I think it's good to fix it, especially if you plan > to enable the feature by default at some point. Okay, we will file a followup to get it fixed in 50. Thanks!

Doesn't need uplift.

(In reply to Tanvi Vyas - behind on reviews [:tanvi] from comment #20) > Okay, we will file a followup to get it fixed in 50. Thanks! https://bugzilla.mozilla.org/show_bug.cgi?id=1285853