Opening e.g. the history subview, some of its items are cut off. Things like the gecko profiler subview also get cut off. Furthermore, if you open a subview, unlike in the main app menu, there is no styling that arranges for the user to be able to see the item they clicked with a blue highlight and a 'back' style arrow. I'm tempted to ifdef out the panelmultiview, especially if that also fixes bug 1370967, and then swap it over to a photonpanelmultiview in the hopes that that will fix some of this stuff for free (and if not, we can fix it properly that way). Either way, this can't ship to release with 55 in its current state, and it will if we don't do something (this isn't behind the photon structure pref).

I noticed that the summary here says this needs a minimum width. I'm also happy to add that here, though I guess we need some design guidance in that case, and one could argue it's part of bug 1354086.

Comment on attachment 8875765 [details] Bug 1370986 - disable panelmultiview in the overflow panel off-nightly for 55, https://reviewboard.mozilla.org/r/147190/#review151828 LGTM! Thanks.

Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/71da3a25b572 switch to photonpanelmultiview for photon, disable off-nightly, r=mikedeboer

Backed out for timing out in browser_photon_customization_context_menus.js: https://hg.mozilla.org/integration/autoland/rev/ec9d6a6c30ee4f72a279725b882609e18e637100 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=71da3a25b5720039ef7a0c84e0b49a35c86cae0a&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=105922752&repo=autoland [task 2017-06-09T19:13:30.836141Z] 19:13:30 INFO - TEST-PASS | browser/components/customizableui/test/browser_photon_customization_context_menus.js | Should be in navbar - [task 2017-06-09T19:13:30.838150Z] 19:13:30 INFO - Leaving test bound [task 2017-06-09T19:13:30.841528Z] 19:13:30 INFO - Entering test bound [task 2017-06-09T19:13:30.844655Z] 19:13:30 INFO - Console message: 1497035430489 addons.xpi WARN Attempting to activate an already active default theme [task 2017-06-09T19:13:30.847053Z] 19:13:30 INFO - TEST-PASS | browser/components/customizableui/test/browser_photon_customization_context_menus.js | new-window-button was found - [task 2017-06-09T19:13:30.849555Z] 19:13:30 INFO - Buffered messages logged at 19:11:59 [task 2017-06-09T19:13:30.853190Z] 19:13:30 INFO - Longer timeout required, waiting longer... Remaining timeouts: 1 [task 2017-06-09T19:13:30.855104Z] 19:13:30 INFO - Buffered messages finished [task 2017-06-09T19:13:30.857515Z] 19:13:30 INFO - TEST-UNEXPECTED-FAIL | browser/components/customizableui/test/browser_photon_customization_context_menus.js | Test timed out -

Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/79cbec4c644c disable panelmultiview in the overflow panel off-nightly for 55, r=mikedeboer

(In reply to Pulsebot from comment #8) > Pushed by gijskruitbosch@gmail.com: > https://hg.mozilla.org/integration/autoland/rev/79cbec4c644c > disable panelmultiview in the overflow panel off-nightly for 55, r=mikedeboer So, the issue that led to the backout is that in some cases, it was possible for the photon panel multiview to cache a height of 0. When running the browser_photon_customization_context_menus.js test, it tried to open the context menu on an item but because the viewContainer had no height, there was nothing to context-click, which broke the test. I didn't see this when writing the patch because I ran the test separately, and in that case it passed. Specifically, if running the browser/components/customizableui/test/browser_overflow_use_subviews.js test first, where we add, use and then remove an item from the panel, we end up with style="height: 0" cached on the viewContainer of the photonpanelmultiview. This is a bug in the Photon panelmultiview implementation. I expect it might be fixed by bug 1369095 and friends, but I don't know if that will land in time for 55 branching, and it didn't seem like something we'd bother uplifting given we have no intention of shipping photon with 55. So instead, I verified that the tests passed even when run together, when continuing to use a panelmultiview (which is expected) and then landed the change again.

(In reply to :Gijs from comment #0) > Furthermore, if you open a subview, unlike in the main app menu, there is no > styling that arranges for the user to be able to see the item they clicked > with a blue highlight and a 'back' style arrow. > Can you elaborate on what this means in terms of verification? Thanks!

(In reply to Grover Wimberly IV [:Grover-QA] from comment #11) > (In reply to :Gijs from comment #0) > > > Furthermore, if you open a subview, unlike in the main app menu, there is no > > styling that arranges for the user to be able to see the item they clicked > > with a blue highlight and a 'back' style arrow. > > > > Can you elaborate on what this means in terms of verification? Thanks! The fix here disabled photon sturff in 55 and 56. If you put an item that opens a subview in the old hamburger panel (e.g.: bookmarks, history, synced tabs, developer tools) into the overflow panel instead (by having it in the toolbar and making the window narrow), it shouldn't open a subview on 55/56 - either it should have fallback behaviour, or it should open a panel anchored on the overflow chevron button. This bug and fix don't affect 57. Does that help?

Verified on 55/56 for Windows/Mac/Ubuntu.