109593 - Back out "full screen" mode

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[Hixie (not reading bugmail)]

Some code was recently checked in under the auspices of bug 68136, offering a 
"full screen" mode for the web browser.

This "full screen" mode does not actually cause Mozilla to go into full screen
mode, it merely hides some of the toolbars, the menu bar, the sidebar, and the
status bar. The caption remains, the window does not take up the full screen.
Even maximising the window does not make it take the full screen.

There are other problems, for example (unlike IE) there is no way to enable
the menu bar, the location bar, or any of the toolbars while in "full screen"
mode. There is no feedback when loading pages (no throbber and no progress
meter, IE has both of these). One toolbar remains (with only 4 buttons, and no
location bar, making it hard to seriously use this mode to browse the web). The
tabs remain, and there is no way to autohide either the tabs nor the navigation
bar, so even with the window maximised and the window border chrome removed by
the window manager, and all but one tab closed, there is no way to show a web
page without Mozilla chrome present.

In addition there are some problems with the way this mode was architected. For
example, the status of the toolbars are stored in <data> elements that increase
the size of every browser window's content model, slowing startup time and
increasing our memory usage. Also, the states are just toggled when switching
modes, so for example opening the sidebar while the "full screen" mode is
enabled will cause it to close when closing "full screen" mode and reopen when
switching back to "full screen" mode.

I propose that this "full screen" mode be backed out.

Comment 1

[Hixie (not reading bugmail)]

Another problem is that this "full screen" mode does not hide toolbars that have
been installed but are not part of the default installation (e.g. the mozgest
toolbar, or the user agent toolbar).

Comment 2

[Jason Kersey]

absolutely useless functionality.

Comment 3

[Alex Bishop]

Note that bug 109603 has been filed, requesting a 'real' full screen mode.

Comment 4

[Håkan Waara]

I agree with the mailnews module owners in this respect: Why do we keep 
implementing all these new silly features, and check them in half-done?  How 
about fixing the thousands of bugs that are reported instead of spending energy 
on new stuff, that will require more time and spawn new bugs? My 2 SEK.

Comment 5

[Peter Trudelle]

Let's not confuse the intent of this bug with disdain for a full-screen mode in
general.  I think that what many call 'silly features' are just features they
personally don't use.  Those that benefit large numbers of other users are not
silly.

Comment 6

[Diego Biurrun]

While I completely agree that this feature should rather be called "full window"
mode than "full screen" mode and I agree that it still has bugs and would much
prefer to have a true "full screen" mode, I strongly oppose the idea of backing
this out. Under Linux I can easily set the window decoration to none, maximise
the window and effectively have mozilla running in fullscreen mode. If I let the
window manager remember the window state, I can keep it that way. Nevertheless
this feature should not be referred to as "full screen" mode and maybe disabled
for the next milestone if some of its bugs do not get fixed. I could easily come
up with a patch for that.

Comment 7

[[e-mail]]

I agree. Just add/remove chromehidden="fullscreen" to the window and use CSS to
hide the parts of the UI that we don't want.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I would like to add my strong support for at least turning this feature off
until it's ready.  I just tried the feature to see what it did, and I then had
to spend 20 minutes to try to figure out how to get out of it so that I could
use the browser again (since my existing browser window and any new browser
window I opened, even after exiting the browser, were all in "full screen"
mode).  I only figured out that the magic key was "F11" (which I didn't notice
carefully enough when I selected the item from the view menu) after querying
bonsai for the checkin of the feature and reading the source.

If this does become a real full screen mode, which I think would be useful
(especially if it applies 'projection' stylesheets instead of 'screen' ones, as
Opera does), it should be important to have obvious ways of figuring out how to
exit.  Admittedly, the shortcut key in the initial menu should be noticeable,
but it can also be hit by accident.  The only discoverable UI I can think of in
this mode would be the context menu, so I'd think there should be a context menu
item for exiting full screen mode, although perhaps there should also be some
visible exit button in the corner.

In summary, I think this should at least be turned off until there's an easily
discoverable way to get out of it for any users who accidentally enter it.

Comment 9

[nnooiissee]

i would have to agree that this implementation bites (and will stand by that
until it works on mac (os x), no menubar, no window dressing).

as to being discoverable, give it a warning dialog first: "now entering full
window mode. press f11 to disable.". make users click that whenever entering
whether by menu or button.

Comment 10

[Diego Biurrun]

David, you stumbled across a known bug:

bug 109585 (Exit Minimal Chrome mode button does not appear in Modern skin)

If you try again with classic skin you will see a button labeled "Normal Screen"
to get back out of the mode.

There are three more bugs filed against "full window" mode that I am aware of:

bug 109586 (No Throbber or Status in Minimal Chrome mode)
bug 109584 (Ctrl+L in minimal chrome mode does nothing)
bug 109627 (disable full-screen mode (f11) fails) - focus is lost, you have to
click the content area once

There is also a proposal to include a slimline version of the URL bar into the
"full window" navigation toolbar. If these bugs get fixed we should have a
usable feature that some users like. No need to rip it out completely, this is
laying the groundwork for a future true full screen mode.

As a compromise, why not make this a pref? It could be toggled by a checkbox in
the preferences panel or by adding a hidden pref to prefs.js. I am willing to
code this if need arises.

This is not that bad. Please do not let your disappointment about not getting a
true full screen mode cloud your vision. And do not forget that this can be a
true full screen mode under Unix.

Comment 11

[Nick Pietraniec]

For what it's worth, I completely agree with Diego...

Comment 12

[Blake Ross]

Prefs are not compromises for unfinished features.  And no, as implemented this
is not laying the groundwork for a future more complete mode, as a much simpler
solution was proposed that is not related to the code in the tree.

Comment 13

[Alec Flett]

actually, prefs are often used to hide unfinished features....I can name a ton
right off the bat: disk/memory cache, new view manager, compose window
recycling, xul cache, etc etc.

There are pleanty of features that have gone in "unfinished" - this turns out to
be one of them. Instead of bitching about how broken it is, file some bugs and
get it fixed. Backing out is for total disagreement on whether we should have a
feature, or when you have smoketest blockers..

The solution I suggested was long after the reviews and super reviews. If
anything, there should be a bug about using that mechanism, _if_ _everyone_
_involved_ agrees it was better. Nobody actually agreed my proposed solution was
better (not even the people bitching here)

Comment 14

[Hixie (not reading bugmail)]

> actually, prefs are often used to hide unfinished features... 

This should not happen. Have you seen how many prefs we ship with? It's an
embarrassment and is making it near-impossible for QA to smoketest all likely 
configurations, let alone all _properly_ test all _possible_ configurations.


> I can name a ton right off the bat: disk/memory cache, new view manager, 
> compose window recycling, xul cache, etc etc.

All of which were checked in before we started using experimental builds for
this kind of thing.


> There are pleanty of features that have gone in "unfinished" - this turns out 
> to be one of them.

This isn't (intended to be) unfinished. Look at the original bug -- Steve even 
marked the bug fixed, and did not open any bugs about ongoing work. Almost none
of the bugs filed about this issue address the fundamental design problems I
listed in the initial comments of this bug. And there is no indication anywhere
that my comments are going to be addressed before the current code ends up in a
shipping product. This is why I strongly believe the code should be backed out.


> Instead of bitching about how broken it is

I am not "bitching", I have explained very carefully why this feature, *as
designed*, is fundamentally flawed.

Comment 15

[Blake Ross]

Well, first, I was talking about a pref with UI, which was one of the proposals. 
That of course would make no sense.

I could understand if there was a hidden pref for this if work was ongoing.  
But as Hixie pointed out, this was intended to be the finished product, save for 
the pending toolbar icon work.

And actually, the people "bitching" here did all agree on (a derivative of) your 
solution.  Ben, Hixie, hyatt and I discussed it.

I disagree that backing out is only for completely unwanted features or 
smoketest blockers.  The navigator module owners agree that, if just for the 
reason that a cleaner solution has been proposed, the current code should be 
removed from the tree.  I don't see why it has to be a religious debate.

Comment 16

[Diego Biurrun]

For the sake of having the complete discussion in this bug, I am pasting Alec
Flett's comment from bug 68136 here:

Well, after some discussion on IRC, I've changed my stance on this. I think that
we should back at least part of this out for two reasons:
1) my proposed solution to the chrome-hiding issue should be hashed out more
2) the Full Screen menu item is decieving.. we should not check in the menu item
until we have more of this feature landed, at least as minimal as maximizing the
window.

That said, I don't think lack of toolkit support should prevent the some amount
of chrome-hiding code from landing, because
1) the chrome-hiding has to land sometime
2) I think that a hidden-chrome, maximized-window state is at least useful to
people looking to go into a maximum-viewable-area mode.

So my proprosed solutions: (take your pick, I support them all)
1) back this whole thing out
2) check in a thing that maximizes the window when going into fullscreen mode

and in either case, we hash out the CSS solution that I proposed.

Comment 17

[Hixie (not reading bugmail)]

What's the status of this bug? I didn't file it to be fixed during the "Future"
milestone. I filed it to be fixed now. None of the issues I mentioned have been
resolved. There is code that fixes almost all of them in the original bug.

Clearing Target Milestone field to trigger re-evaluation.

Comment 18

[Stephen P. Morse]

Only the assignee is permitted to change the target milestone field.

Comment 19

[Nick Pietraniec]

But shouldn't this be resolved before the next Mozilla release?  

Yes, it will be removed or no it won't...  Isn't that the point?

Comment 20

[Hixie (not reading bugmail)]

morse:
> Only the assignee is permitted to change the target milestone field.

I was told by members of [e-mail] that the way to trigger reevaluation
is to clear the target milestone field, my apologies for any offence. How do you
prefer to have your bugs marked for reevaluation?

Comment 21

[[e-mail]]

Attachment: CSS solution (chrome)

This is a patch I wrote to move most of the fullscreen work into CSS.
The only JS code left is for the toggle function.
No extra work is needed to hide 3rd party toolbars.
The nav buttons should show even if the toolbar is collapsed or hidden.
On my 133MHz 32MB notebook the speed increase is noticable.

Comment 22

[[e-mail]]

Attachment: Patch to classic skin to hide icons in fullscreen mode

I have hidden the throbber in these patches but a "small" throbber could be
used.

Comment 23

[:deanis74]

This bug isn't for fixing this problem, it's for backing it out.

ps. Ben's "fifth patch" in bug 68136 (attachment 57717 [details] [diff] [review]) moves to css as well.

Comment 24

[Diego Biurrun]

*** Bug 120024 has been marked as a duplicate of this bug. ***

Comment 25

[Peter Trudelle]

retargetting, since this feature has been cut from our schedule, and we need to
back it out until we can get back to finish it.

Comment 26

[Stephen P. Morse]

Attachment: reverse 11-8-2001 patch from bug 68136

Comment 27

[Stephen P. Morse]

alecf, sgehani please review.  Thanks.

Comment 28

[Stephen P. Morse]

Note: final quote is missing on <window> tag of navigator.xul.  Corrected final 
line of that tag should be:

          persist="screenX screenY width height sizemode"> 

Comment 29

[Diego Biurrun]

Attachment: patch that renames menu entry to "Full Window"

Instead of removing this functionality that some of us like and can be made
into a full screen mode on Unix I propose to rename the menu entry to "Full
Window".  That way we will not create false expectations and those that like
the feature can continue using it.

Comment 30

[nnooiissee]

this is not even a full window mode as it leaves a full size (but not full
functionality) navigation bar, and the tabs bar (which would in most useful
situations not be present).

for a full window mode check out the mac os x build, which uses os x's
toolbar-button-thing to hide everything except tabs and sidebar.

if this stays in, or ever makes it back in, how about making the nav bar
vertical to take up less (and less important) screen real estate?

Comment 31

[Diego Biurrun]

If you disable the navigation toolbar you get a complete full window mode.

Comment 32

[Peter Trudelle]

Diego, please don't try to morph this bug.  If you want a 'full window mode',
whatever that is, please file a separate enhancement bug for it.

Comment 33

[Diego Biurrun]

I'm not trying to morph this bug.  Here is where the future of the "minimal
chrome mode" gets decided.  If I file a separate bug, it will sit there
unnoticed like thousands of its peers and this will get backed out.  It would
have to get checked  back in to be reenabled.  That would be patently absurd.

Comment 34

[Peter Trudelle]

This bug is for backing out a partially implemented feature, not for deciding
the future of 'minimal chrome mode', or full screen mode, or anything else.  We
are not going to allow it to turn into an enhancement request for some other
feature, regardless of how much code you think they would have in common.  This
will be backed out.

Comment 35

[Diego Biurrun]

Comment on attachment 65114 [details] [diff] [review]
patch that renames menu entry to "Full Window"

Go ahead then, back it out.

'Minimal chrome mode' and 'full window mode' were just names this feature got
called to avoid the misleading term 'full screen mode'.

Comment 36

[Samir Gehani]

Comment on attachment 65077 [details] [diff] [review]
reverse 11-8-2001 patch from bug 68136

r=sgehani

Comment 37

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 65077 [details] [diff] [review]
reverse 11-8-2001 patch from bug 68136

since this feature has been cut...

sr=[e-mail]

Comment 38

[Asa Dotzler [:asa]]

*** Bug 120024 has been marked as a duplicate of this bug. ***

Comment 39

[Stephen P. Morse]

checked in

Comment 40

[:deanis74]

This should have been verified a long time ago.