Make fullscreen mode really fullscreen

ASSIGNED
Assigned to

Status

defect
ASSIGNED
9 years ago
3 years ago

People

(Reporter: mnyromyr, Assigned: mnyromyr)

Tracking

(Depends on 1 bug, Blocks 2 bugs)

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Bug 606297 will enable full screen mode on Mac as such, but minimal urlbar etc. are still visible. Those need to hide as well (and return eg. when hitting the top screen edge with the mouse).
Not only a Mac problem.
OS: Mac OS X → All
Hardware: x86_64 → All
Summary: Make Mac full screen mode really full screen → Make fullscreen mode really fullscreen
This patch mainly replaces our code in fullscreen.js by the corresponding code from FF's browser.js, with/plus changes to suit the suite.

That means:
- on F11/Cmd-Shift-F only the special fullscreen urlbar remains, vanishing slowly into the upper screen frame
- on Accel-L or moving the mouse against the top screen frame, it'll appear again

Note that it won't autohide if the current tab is empty (on purpose, I suppose).
Assignee: nobody → mnyromyr
Attachment #565724 - Flags: superreview?(neil)
Attachment #565724 - Flags: review?(stefanh)
Attachment #565724 - Flags: feedback?(philip.chee)
I noticed at least one non-Mac bug, which is that the menubar doesn't work.
(In reply to neil@parkwaycc.co.uk from comment #3)
> I noticed at least one non-Mac bug, which is that the menubar doesn't work.

Can you elaborate, please? 
Everything I tested did work (of course ^_^).
On mac, if I have a few tabs opened and hit fullscreen mode, I don't see the tab bar. I just see the url bar and the nav-bar buttons.
Comment on attachment 565724 [details] [diff] [review]
port Firefox fullscreen code, v1

+    // In tabs-on-top mode, move window controls to the tab bar,
+    // and in tabs-on-bottom mode, move them back to the navigation toolbar.
+    // When there is a chance the tab bar may be collapsed, put window
+    // controls on nav bar.
+    var fullscreenflex = document.getElementById("fullscreenflex");
+    var fullscreenctls = document.getElementById("window-controls");
+    var navbar = document.getElementById("nav-bar");
+    var ctlsOnTabbar = window.toolbar.visible && navbar.collapsed;
+    if (fullscreenctls.parentNode == navbar && ctlsOnTabbar) {
+      document.getElementById("TabsToolbar").appendChild(fullscreenctls);
+      // we don't need this space in tabs-on-top mode, so prevent it from 
+      // being shown
+      fullscreenflex.removeAttribute("fullscreencontrol");
+    }
+    else if (fullscreenctls.parentNode.id == "TabsToolbar" && !ctlsOnTabbar) {
+      navbar.appendChild(fullscreenctls);
+      fullscreenflex.setAttribute("fullscreencontrol", "true");
+    }
+
+    var controls = document.getElementsByAttribute("fullscreencontrol", "true");
+    for (var i = 0; i < controls.length; ++i)
+      controls[i].hidden = aShow;
   }

No tabs-on-top functionality in SM :-)

+#fullscr-toggler {
+  height: 1px;
+  background: black;

Nit: hex values are nicer (do we need a colour here btw?).
Another nit: Mixing let and var.
Some preliminary comments while I try fix the issues.

(In reply to neil@parkwaycc.co.uk from comment #3)
> I noticed at least one non-Mac bug, which is that the menubar doesn't work.

JFTR: This means that if in fullscreen mode, Alt-F doesn't open/show the File menu, etc.

(In reply to Stefan [:stefanh] from comment #5)
> On mac, if I have a few tabs opened and hit fullscreen mode, I don't see the
> tab bar. I just see the url bar and the nav-bar buttons.

That's on purpose, or rather: that's a known glitch.
Actually, the tabbar should scroll/hide away with the rest of the toolbars, but our tabbar isn't a toolbar yet. I'll have a look.

(In reply to Stefan [:stefanh] from comment #6)
> No tabs-on-top functionality in SM :-)

Yet. ;-)
Throwing it out now may mean to have to ponder on how to get it in again once we implement tabs-on-top. How about just commenting out?

> +  background: black;
> 
> Nit: hex values are nicer

Well, I disagree, but I don't care enough to argue. ;-)

> (do we need a colour here btw?).

Screen edges are usually black, so it won't show, I guess. 
Maybe transparent works as well.
Depends on: 428981
Status: NEW → ASSIGNED
Blocks: 199387
Note bug 639705 which will add Lion-specific fullscreen support.
Attachment #565724 - Flags: superreview?(neil)
Attachment #565724 - Flags: review?(stefanh)
Attachment #565724 - Flags: feedback?(philip.chee)
You need to log in before you can comment on or make changes to this bug.