Closed Bug 109593 Opened 23 years ago Closed 23 years ago

Back out "full screen" mode

Categories

(Core :: XUL, defect)

defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla0.9.9

People

(Reporter: ian, Assigned: morse)

References

Details

Attachments

(1 file, 3 obsolete files)

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.
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).
absolutely useless functionality.
Note that bug 109603 has been filed, requesting a 'real' full screen mode.
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.
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.
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.
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.
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.
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.
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.
For what it's worth, I completely agree with Diego...
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.
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)
Status: NEW → ASSIGNED
Target Milestone: --- → Future
> 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.
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.
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.
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.
Severity: normal → major
Keywords: mozilla0.9.7
Target Milestone: Future → ---
Only the assignee is permitted to change the target milestone field.
Target Milestone: --- → Future
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?
morse: > Only the assignee is permitted to change the target milestone field. I was told by members of stafff@mozilla.org 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?
Attached patch CSS solution (chrome) (obsolete) (deleted) — Splinter Review
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.
I have hidden the throbber in these patches but a "small" throbber could be used.
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.
*** Bug 120024 has been marked as a duplicate of this bug. ***
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.
Target Milestone: Future → mozilla0.9.8
Attachment #59313 - Attachment is obsolete: true
Attachment #59316 - Attachment is obsolete: true
alecf, sgehani please review. Thanks.
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">
Attached patch patch that renames menu entry to "Full Window" (obsolete) (deleted) — Splinter Review
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.
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?
If you disable the navigation toolbar you get a complete full window mode.
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.
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.
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 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'.
Attachment #65114 - Attachment is obsolete: true
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Comment on attachment 65077 [details] [diff] [review] reverse 11-8-2001 patch from bug 68136 r=sgehani
Attachment #65077 - Flags: review+
Comment on attachment 65077 [details] [diff] [review] reverse 11-8-2001 patch from bug 68136 since this feature has been cut... sr=ben@netscape.com
Attachment #65077 - Flags: superreview+
*** Bug 120024 has been marked as a duplicate of this bug. ***
checked in
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
This should have been verified a long time ago.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: