I think UI-initiated full screen browsing can probably leave the social sidebar in place, but DOM-initiated fullscreen (e.g. when playing a bananabread demo) should almost certainly hide it.
I think we need to fix this for v1.
Created attachment 664297 [details] [diff] [review] Patch v.0 This is the easy fix, but has a couple of concerns: * Doesn't differentiate between DOM-fullscreen and browser-fullscreen (ie, F11)... Depending on how you're using F11 mode, you may or may not want the sidebar. We do keep the bookmarks sidebar around, fwiw. * Should I worry about the display:none tearing down bindings and having something unexpectedly fail? I did a quick test with width/max-width set to zero, but that wasn't working.
Hmm, I wonder if this might mess up provider expectations (for the moment, the sidebar is only loaded when it is visible). A simpler solution might be to have the browser-fullscreen code call SocialSidebar.updateSidebar() and add a check there for full screen mode (both types? that's confusing).
Created attachment 667213 [details] [diff] [review] alternate patch This hides the sidebar, without unloading the provider, when in dom fullscreen mode. The sidebar remains visible (if it was) when in browser fullscreen mode.
I also renamed SocialSidebar.enabled since a) it threw me off reading the code, and b) that was about whether the sidebar was toggled open or not.
Created attachment 667723 [details] [diff] [review] alternate patch updated with feedback
Comment on attachment 667723 [details] [diff] [review] alternate patch Is mozfullscreenchange the right event name? browser.js seems to use MozEnteredDomFullscreen. And should you check window.fullScreen in addition to mozFullScreen? As I understand it there are two separate "full screen mode"s (though I'm not quite clear on the difference).
Gavin, mozfullscreenchange is fired when the dom fullscreen mode is entered/adjusted/exited. The other fullscreen mode is F11 full-screen, which IMO we should leave the sidebar visibility unchanged. MozEnteredDomFullscreen implies that it only fires for entering fullscreen, and is only useful if you are displaying a fullscreen warning for security reasons.
As for the flags: window.mozFullScreen, window.mozFullScreenElement and/or window.fullScreen. MDN Documentation is not terribly clear: mozFullScreen: whether document is in full screen mode fullScreen: whether window is in full screen mode mozFullScreenElement: element being presented in full screen mode I *think* mozFullScreen is a flag for DOM full screen, whereas fullScreen is browser full screen, in any case using mozFullScreen is getting the correct results. clear as mud?
Pushed to try server: https://tbpl.mozilla.org/?tree=Try&rev=7c7658690388
No QA verification since this has in-testsuite coverage.