Kevin Ar18 Reporter
	Description • 21 years ago

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4b) Gecko/20030516 Mozilla Firebird/0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4b) Gecko/20030516 Mozilla Firebird/0.6

Would be nice to have the "Full Screen" option from the View menu also available
on the toolbar (through Customize...?)

Reproducible: Always

Steps to Reproduce:
.
Actual Results:  
.

Expected Results:  
.

.

	Kevin Ar18 Reporter
	Comment 1 • 21 years ago

OS -> All

	David P James
	Comment 2 • 21 years ago

Have you considered using the following extension? It adds an item to the
context menu to go to/from Full Screen.

http://texturizer.net/firebird/extensions.html#Autohide%20Toolbox%20%28True%20Fullscreen%29

	Kevin Ar18 Reporter
	Comment 3 • 21 years ago

Thanks, I just tested the extension....
I already had the Full Screen option in the context menu, so that wasn't an
issue in my request.

What I am proposing is making some more icons available for placing in the
toolbar like the fullscreen option.

	Mike Connor [:mconnor]
	Comment 4 • 21 years ago

This is something that would belong more in the linked extension, but the
request seems clear enough.  Confirming and recommending WONTFIX.

	Kevin Ar18 Reporter
	Comment 5 • 21 years ago

Should this be relegated to an extension?  Would it not be prudent to include a
full range of options (buttons) for the user to add to their toolbar?  I was
considering suggesting other icons, that could be available in the cutomize
section.  Then again, maybe even have them as a default.  It might be much more
helpful helpful that way.

	Simon Paquet [:sipaq]
	Comment 6 • 21 years ago

Taking QA Contact

	Stephan T. Lavavej
	Comment 7 • 20 years ago

I was just about to submit a duplicate of this, but wisely checked the 
existing bugs first.

As a user coming from IE this enhancement request is very important to me. If 
Firefox supports Full Screen, it should have a Full Screen button.

	u88484
	Comment 8 • 20 years ago

ya firefox definently needs a full screen icon button

	Tom Londer
	Comment 9 • 20 years ago

Hi Firefox people,
I want to add an icon to the toolbar that allows Full Screen.  I know about the
View Menu and F11. I've searched through tons of FF and Mozilla pages looking
for this.  Please give me a heads up.  I may not be able to figure out how to
see this question as bug report (and it really isn't a bug) - it's just a handy
feature.
PLEASE, if someone has a solution or can "point me" to the right place to learn
how to do this PLEASE email me direct at tlonder@comcast.net.  Will be eternally
grateful (and maybe I can do something FOR YOU!
Thanks,
Tom Londer

	Tom Londer
	Comment 10 • 20 years ago

Bugzilla Bug 206544

full screen toolbar item

Here’s the solution for the above along with a  lot of other useful tools!:

<"http://www.w3.org/TR/html4/strict.dtd">

It’s called toolbar enhancements!  

	Tom Londer
	Comment 11 • 20 years ago

Here's the link:

http://extensionroom.mozdev.org/more-info/tbx

<"http://extensionroom.mozdev.org/more-info/tbx">

	Dan Fischbach
	Comment 12 • 20 years ago

I agree, I only get the Toolbar Enhancements extension just for that button,
which sems silly.  Voted.

	Dan Fischbach
	Comment 13 • 19 years ago

Can this be nominated for 1.1?  IE has this...why doesn't Firefox have this by
default?  It seems silly to get the extension and bloat Firefox just for this
one button.

	Mike Connor [:mconnor]
	Comment 14 • 19 years ago

Not a blocker, will consider a reasonable patch.

	Dave Townsend [:mossop]
	Comment 15 • 19 years ago

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

	Georges-Etienne Legendre
	Comment 16 • 18 years ago

Here is a suggested patch. Of course, a new icon will be required. This patch currently uses the "new-tab" icon.

	Mike Connor [:mconnor]
	Comment 18 • 18 years ago

not a blocker, will consider patch separately.

	Georges-Etienne Legendre
	Comment 19 • 18 years ago

Who is able to review this patch? What's the next step?

	石庭豐 (Seak, Teng-Fong)
	Comment 20 • 18 years ago

Personally, I use this extension (https://addons.mozilla.org/firefox/810/) to have the icon available for customization!

Not only is it quite stupid to install an extension/addon just to have an icon, it's also very tedious for administrators.

I'm in charge of installing O/S to computers for my company (amongst misc many other tasks).  Every time I install a new machine, I'll install Firefox.  Then I have to install the extensions: there're about 7 or 8 of them.  It would be nice if I could install one less.  Well, one extension less is one extension less.  If you have to install, say, one computer everyday, you would be delighted that there's a little bit of work to do!

And there were all those upgrades: from 1.0 to 1.5, then from 1.5 to 2.0.  And every upgrade just breaks this extension and I have to reinstall the extension over again for ALL the machines!  Can you imagine the workload!?

So, I also vote to include the icon in Firefox instead of having us install any extension.

I've changed Version value to 2.0 branch and hardware to ALL.  I know FF in some O/S doesn't work well with full screen mode, but it doesn't hurt to include the icon and hide it in O/S where full screen mode isn't available.

	u88484
	Comment 21 • 17 years ago

Can we get this for Firefox 3? I know people are getting ready to work on the theme so this is idea time to add a new button and get it all in one shot!

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 22 • 17 years ago

Alex, can you add this to the icon inventory requirements?

	Alex Faaborg [:faaborg] (Firefox UX)
	Comment 24 • 16 years ago

Note that we now have the icon in toolbar.png

	u286998
	Comment 25 • 16 years ago

@Georges-Etienne Legendre: You should probably request a review for the patch, i guess Mike Conner is the person to ask here.

	Steffen Wilberg Assignee
	Comment 26 • 15 years ago

The previous patch was bitrotted in every single hunk.
This patch applies to mozilla-central. It uses the winstripe icons we have since Firefox 3.0, and the gtk stock icon for gnomestripe. It fixes a bug in the previous patch which showed a checked state of the button in normal mode instead of in fullscreen mode.

Tested on Windows, needs testing on Linux.

	Steffen Wilberg Assignee
	Comment 27 • 15 years ago

Sorry, this is the one.

	Steffen Wilberg Assignee
	Comment 28 • 15 years ago

Tested on Linux as well.

	Dão Gottwald [:dao]
	Comment 29 • 15 years ago

Can you do that without removing View:FullScreen?

	Steffen Wilberg Assignee
	Comment 30 • 15 years ago

Here you are.

	Dão Gottwald [:dao]
	Comment 31 • 15 years ago

Sorry, I wasn't clear. I actually meant keeping the command, rather than calling the broadcaster View:FullScreen.

	Steffen Wilberg Assignee
	Comment 32 • 15 years ago

I just learned that I can use a command like a broadcaster:
"Broadcasters aren't used frequently as commands can generally handle most uses. One thing to point out is that there really is no difference between the command  element and the broadcaster element. They both do the same thing. The difference is more semantic. Use commands for actions and use broadcasters for state. In fact, any element can act as broadcaster, as long as you observe it using the observes attribute."
https://developer.mozilla.org/en/XUL_Tutorial/Broadcasters_and_Observers

So I can keep the command, but use it like a broadcaster, which I need for the pressed state of the toolbar button (the checkbox of the menu item isn't visible in full screen mode since you can't access the menu bar).

Are you worrying about extension compat?

	Dão Gottwald [:dao]
	Comment 33 • 15 years ago

Comment on attachment 391428 [details] [diff] [review]
keeping the command

>+    <command id="View:FullScreen"
>+             type="checkbox" autoCheck="false"
>+             label="&fullScreenCmd.label;" 
>+             oncommand="BrowserFullScreen();"/>

type, autoCheck and label should be set on the appropriate elements directly rather than on the command element.

>-    <key id="key_fullScreen" keycode="VK_F11" command="View:FullScreen"/>
>+    <key id="key_fullScreen" keycode="VK_F11" observes="View:FullScreen"/>

That change doesn't make sense.

> function BrowserFullScreen()
> {
>   window.fullScreen = !window.fullScreen;
>+  let fullscreenBroadcaster = document.getElementById("View:FullScreen");
>+  fullscreenBroadcaster.setAttribute("checked", window.fullScreen);
> }

This needs to be added in the onFullScreen function.

>+#fullscreen-button {
>+  -moz-image-region: rect(0px 360px 24px 336px);
>+}
>+#fullscreen-button:not([disabled="true"]):hover {
>+  -moz-image-region: rect(24px 360px 48px 336px);
>+}
>+#fullscreen-button[disabled="true"] {
>+  -moz-image-region: rect(48px 360px 72px 336px);
>+}
>+#fullscreen-button:not([disabled="true"]):hover:active {
>+  -moz-image-region: rect(96px 360px 120px 336px);
>+}

Why should that toolbarbutton support the disabled state?

	Steffen Wilberg Assignee
	Comment 34 • 15 years ago

(In reply to comment #33)
> (From update of attachment 391428 [details] [diff] [review])
> >+    <command id="View:FullScreen"
> >+             type="checkbox" autoCheck="false"
> >+             label="&fullScreenCmd.label;" 
> >+             oncommand="BrowserFullScreen();"/>
> 
> type, autoCheck and label should be set on the appropriate elements directly
> rather than on the command element.
Fixed. Note that this duplicates type, autoCheck and label on the menuitem and toolbarbutton.

> >-    <key id="key_fullScreen" keycode="VK_F11" command="View:FullScreen"/>
> >+    <key id="key_fullScreen" keycode="VK_F11" observes="View:FullScreen"/>
> 
> That change doesn't make sense.
I dropped that.

> > function BrowserFullScreen()
> > {
> >   window.fullScreen = !window.fullScreen;
> >+  let fullscreenBroadcaster = document.getElementById("View:FullScreen");
> >+  fullscreenBroadcaster.setAttribute("checked", window.fullScreen);
> > }
> 
> This needs to be added in the onFullScreen function.
Yeah, but FullScreen.toggle(), which is called from onFullScreen(), is even better, because it already set the checked attribute for the menuitem.
Now it sets the attribute on the command, which is observed from both menuitem and toolbarbutton.

> >+#fullscreen-button {
> >+  -moz-image-region: rect(0px 360px 24px 336px);
> >+}
> >+#fullscreen-button:not([disabled="true"]):hover {
> >+  -moz-image-region: rect(24px 360px 48px 336px);
> >+}
> >+#fullscreen-button[disabled="true"] {
> >+  -moz-image-region: rect(48px 360px 72px 336px);
> >+}
> >+#fullscreen-button:not([disabled="true"]):hover:active {
> >+  -moz-image-region: rect(96px 360px 120px 336px);
> >+}
> 
> Why should that toolbarbutton support the disabled state?
Not sure. Why do the home, bookmarks, history, downloads, print, new tab, new window buttons support the disabled state?
Why do those buttons have disabled states in Toolbar.png?

	Dão Gottwald [:dao]
	Comment 35 • 15 years ago

Comment on attachment 391673 [details] [diff] [review]
fixed review comments

>+#ifndef XP_MACOSX
>     <command id="View:FullScreen" oncommand="BrowserFullScreen();"/>
>+#endif

What's that change about?

(In reply to comment #34)
> Not sure. Why do the home, bookmarks, history, downloads, print, new tab, new
> window buttons support the disabled state?
> Why do those buttons have disabled states in Toolbar.png?

Because somebody thought they might be needed, even though they aren't.

	Steffen Wilberg Assignee
	Comment 36 • 15 years ago

(In reply to comment #35)
> (From update of attachment 391673 [details] [diff] [review])
> >+#ifndef XP_MACOSX
> >     <command id="View:FullScreen" oncommand="BrowserFullScreen();"/>
> >+#endif
> 
> What's that change about?
The menuitem, key and the new toolbarbutton are ifndef XP_MACOSX, so why not the command? Nobody's using the command on Mac.

> (In reply to comment #34)
> > Not sure. Why do the home, bookmarks, history, downloads, print, new tab, new
> > window buttons support the disabled state?
> > Why do those buttons have disabled states in Toolbar.png?
> 
> Because somebody thought they might be needed, even though they aren't.
OK, I'll drop it.

	Dão Gottwald [:dao]
	Comment 37 • 15 years ago

(In reply to comment #36)
> The menuitem, key and the new toolbarbutton are ifndef XP_MACOSX, so why not
> the command? Nobody's using the command on Mac.

Extensions could.

	Steffen Wilberg Assignee
	Comment 38 • 15 years ago

OK, I left the command alone and dropped the disabled state of the toolbarbutton.

	Dão Gottwald [:dao]
	Comment 39 • 15 years ago

Comment on attachment 391691 [details] [diff] [review]
fixed further comments

>                 <menuitem id="fullScreenItem"
>                           accesskey="&fullScreenCmd.accesskey;"
>                           label="&fullScreenCmd.label;"
>                           key="key_fullScreen"
>                           type="checkbox"
>-                          command="View:FullScreen"/>
>+                          autoCheck="false"
>+                          observes="View:FullScreen"/>

autoCheck="false" isn't really needed, is it?

>+#ifndef XP_MACOSX
>+        <toolbarbutton id="fullscreen-button" class="toolbarbutton-1 chromeclass-toolbar-additional"
>+                       observes="View:FullScreen"
>+                       label="&fullScreenCmd.label;"
>+                       tooltiptext="&fullScreenCmd.label;"/>
>+#ifndef XP_MACOSX

Looking at the other toolbarbuttons, it looks like you shouldn't use &fullScreenCmd.label; as the tooltiptext but add a new entity. This is the only significant remaining issue, sorry for not noticing earlier.

>+toolbar[iconsize="small"] #fullscreen-button > .toolbarbutton-icon {
>+  padding-left: 1px;
>+}

We seem to do this for all small toolbarbuttons, but I don't understand why. :(
Do you happen to know?

	Steffen Wilberg Assignee
	Comment 40 • 15 years ago

(In reply to comment #39)
> (From update of attachment 391691 [details] [diff] [review])
> >                 <menuitem id="fullScreenItem"
> >+                          autoCheck="false"
> 
> autoCheck="false" isn't really needed, is it?
I don't think so, no. At least I can't check whether the checkbox is set on the menuitem in full screen mode since I can't access the menu bar there.

> Looking at the other toolbarbuttons, it looks like you shouldn't use
> &fullScreenCmd.label; as the tooltiptext but add a new entity. This is the only
> significant remaining issue, sorry for not noticing earlier.
Fixed.

> >+toolbar[iconsize="small"] #fullscreen-button > .toolbarbutton-icon {
> >+  padding-left: 1px;
> >+}
> 
> We seem to do this for all small toolbarbuttons, but I don't understand why. :(
> Do you happen to know?
Nope. Mano added that in bug 353673 without explanation. Maybe the buttons are too squeezed together without it, or it interferes with the hover box. I didn't try to remove it though.

	Dão Gottwald [:dao]
	Comment 41 • 15 years ago

Comment on attachment 391721 [details] [diff] [review]
with nicer tooltip

>+        <toolbarbutton id="fullscreen-button" class="toolbarbutton-1 chromeclass-toolbar-additional"
>+                       observes="View:FullScreen"
>+                       label="&fullScreenCmd.label;"
>+                       tooltiptext="&fullScreenButton.tooltip;"/>

Seems like you missed type="checkbox".

Looks good otherwise. The new string needs ui-review.

	Dão Gottwald [:dao]
	Comment 42 • 15 years ago

(In reply to comment #40)
> Mano added that in bug 353673 without explanation. Maybe the buttons are
> too squeezed together without it, or it interferes with the hover box.

The icons look misaligned over here because of that padding. We need to remove it, I think, but that can happen in a separate bug.

	Steffen Wilberg Assignee
	Comment 43 • 15 years ago

Comment on attachment 391721 [details] [diff] [review]
with nicer tooltip

Alex, this patch adds an optional "full screen" toolbar button. We already have the icon in Toolbar.png, as you noted 16 months ago in comment 24.

Requesting ui-review on its tooltip "Display the window in full screen". Thanks.

	Jennifer Morrow [:Boriss] (UX)
	Comment 44 • 15 years ago

Comment on attachment 391721 [details] [diff] [review]
with nicer tooltip

ui+

	Dão Gottwald [:dao]
	Comment 46 • 15 years ago

http://hg.mozilla.org/mozilla-central/rev/702e686f3bcd

	Dão Gottwald [:dao]
	Comment 47 • 15 years ago

and http://hg.mozilla.org/mozilla-central/rev/94cd140316fb

	Steffen Wilberg Assignee
	Comment 48 • 15 years ago

Thanks for the quick reviews and the pushes, and sorry for the bustage.

	Steffen Wilberg Assignee
	Comment 49 • 15 years ago

Might want to announce this in the release notes since it's hidden in the customize dialog by default.

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 50 • 15 years ago

Verified fixed on 1.9.2 for builds on all platforms like Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b1pre) Gecko/20090922 Namoroka/3.6b1pre ID:20090922041132

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 51 • 15 years ago

Removing relnote; called out as a new feature.

	Mike Kaply [:mkaply]
	Comment 52 • 15 years ago

did we have Mac fullscreen before this?

Should we also be calling out that we have mac fullscreen support now?

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 53 • 15 years ago

(In reply to comment #52)
> did we have Mac fullscreen before this?

We had until 1.0 or 1.5.

> Should we also be calling out that we have mac fullscreen support now?

We haven't done that in the beta release notes? See bug 505699 where we put a relnote on it and which has been removed by Beltzner. Please follow up there or with a message in the newsgroup. It's nothing for this bug.

	Steffen Wilberg Assignee
	Comment 54 • 15 years ago

No, neither this bug (add an optional Full Screen toolbar button) nor bug 505699 (make full screen work on OS X at all) are mentioned in the release notes for 3.6b1. Full Screen *video* is mentioned, but that's not the same.

	Chris Ilias [:cilias]
	Comment 55 • 15 years ago

user-doc-complete
https://support.mozilla.com/en-US/kb/Navigation+Toolbar+items