Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Include "Full Screen" button in toolbar customize menu

VERIFIED FIXED in Firefox 3.6a1

Status

()

Firefox
Toolbars and Customization
--
enhancement
VERIFIED FIXED
14 years ago
8 years ago

People

(Reporter: Kevin Ar18, Assigned: Steffen Wilberg)

Tracking

({user-doc-complete, verified1.9.2})

Trunk
Firefox 3.6a1
user-doc-complete, verified1.9.2
Points:
---
Dependency tree / graph
Bug Flags:
blocking-aviary1.5 -
blocking-firefox3 -
wanted-firefox3 +
blocking1.9a1 -
blocking-firefox2 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 9 obsolete attachments)

(Reporter)

Description

14 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:  
.

.
(Reporter)

Comment 1

14 years ago
OS -> All
OS: Windows XP → All

Comment 2

14 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
Summary: Include "Full Screen" option in menu → Include "Full Screen" button in toolbar customize menu
(Reporter)

Comment 3

14 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.
This is something that would belong more in the linked extension, but the
request seems clear enough.  Confirming and recommending WONTFIX.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 5

14 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.

Comment 6

14 years ago
Taking QA Contact
QA Contact: asa → bugzilla

Comment 7

13 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.

Comment 8

13 years ago
ya firefox definently needs a full screen icon button

Comment 9

13 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

Comment 10

13 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!  

Comment 11

13 years ago
Here's the link:

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

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

Comment 12

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

Comment 13

12 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.

Updated

12 years ago
Flags: blocking-aviary1.1?
Not a blocker, will consider a reasonable patch.
Flags: blocking-aviary1.1? → blocking-aviary1.1-

Updated

12 years ago
Flags: blocking1.9a1?
Flags: blocking1.9a1? → blocking1.9a1-

Updated

12 years ago
Assignee: hyatt → nobody
QA Contact: bugzilla → toolbars
*** Bug 318930 has been marked as a duplicate of this bug. ***
Created attachment 224285 [details] [diff] [review]
Suggested patch

Here is a suggested patch. Of course, a new icon will be required. This patch currently uses the "new-tab" icon.
Flags: blocking-firefox2?
Created attachment 224301 [details] [diff] [review]
Suggested patch with "checkbox" style button
Attachment #224285 - Attachment is obsolete: true
not a blocker, will consider patch separately.
Flags: blocking-firefox2? → blocking-firefox2-
Who is able to review this patch? What's the next step?
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.
Hardware: PC → All
Version: unspecified → 2.0 Branch

Comment 21

10 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!
Flags: blocking-firefox3?
Keywords: uiwanted
Alex, can you add this to the icon inventory requirements?
Flags: blocking-firefox3? → blocking-firefox3-
Whiteboard: [wanted-firefox3]
Flags: wanted-firefox3+
Whiteboard: [wanted-firefox3]

Updated

9 years ago
Duplicate of this bug: 425670
Note that we now have the icon in toolbar.png
Blocks: 425582

Updated

9 years ago
Version: 2.0 Branch → Trunk

Comment 25

9 years ago
@Georges-Etienne Legendre: You should probably request a review for the patch, i guess Mike Conner is the person to ask here.
Flags: wanted-firefox3.1?
(Assignee)

Comment 26

8 years ago
Created attachment 390727 [details] [diff] [review]
unbitrotted and fixed patch

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.
Assignee: nobody → steffen.wilberg
Attachment #224301 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #390727 - Flags: review?(dao)
(Assignee)

Comment 27

8 years ago
Created attachment 390728 [details] [diff] [review]
correct patch

Sorry, this is the one.
Attachment #390727 - Attachment is obsolete: true
Attachment #390728 - Flags: review?(dao)
Attachment #390727 - Flags: review?(dao)
(Assignee)

Updated

8 years ago
Flags: wanted-firefox3.5? → wanted-firefox3.6?
Keywords: uiwanted
(Assignee)

Comment 28

8 years ago
Tested on Linux as well.
Can you do that without removing View:FullScreen?
(Assignee)

Comment 30

8 years ago
Created attachment 391323 [details] [diff] [review]
keeping "View:FullScreen"

Here you are.
Attachment #390728 - Attachment is obsolete: true
Attachment #391323 - Flags: review?(dao)
Attachment #390728 - Flags: review?(dao)
Sorry, I wasn't clear. I actually meant keeping the command, rather than calling the broadcaster View:FullScreen.
(Assignee)

Comment 32

8 years ago
Created attachment 391428 [details] [diff] [review]
keeping the command

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?
Attachment #391323 - Attachment is obsolete: true
Attachment #391428 - Flags: review?(dao)
Attachment #391323 - Flags: review?(dao)
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?
Attachment #391428 - Flags: review?(dao) → review-
(Assignee)

Comment 34

8 years ago
Created attachment 391673 [details] [diff] [review]
fixed review comments

(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?
Attachment #391428 - Attachment is obsolete: true
Attachment #391673 - Flags: review?(dao)
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.
(Assignee)

Comment 36

8 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.
(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.
(Assignee)

Comment 38

8 years ago
Created attachment 391691 [details] [diff] [review]
fixed further comments

OK, I left the command alone and dropped the disabled state of the toolbarbutton.
Attachment #391673 - Attachment is obsolete: true
Attachment #391691 - Flags: review?(dao)
Attachment #391673 - Flags: review?(dao)
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?
(Assignee)

Comment 40

8 years ago
Created attachment 391721 [details] [diff] [review]
with nicer tooltip

(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.
Attachment #391691 - Attachment is obsolete: true
Attachment #391721 - Flags: review?(dao)
Attachment #391691 - Flags: review?(dao)
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.
Attachment #391721 - Flags: review?(dao) → review+
(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.
(Assignee)

Comment 43

8 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.
Attachment #391721 - Flags: ui-review?(faaborg)
(Assignee)

Updated

8 years ago
Whiteboard: [need to fix comment 41]
Comment on attachment 391721 [details] [diff] [review]
with nicer tooltip

ui+
Attachment #391721 - Flags: ui-review?(faaborg) → ui-review+
(Assignee)

Comment 45

8 years ago
Created attachment 391889 [details] [diff] [review]
with comment 41 addressed
Attachment #391721 - Attachment is obsolete: true
Attachment #391889 - Flags: ui-review+
Attachment #391889 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/702e686f3bcd
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Whiteboard: [need to fix comment 41]
Target Milestone: --- → Firefox 3.6a1
and http://hg.mozilla.org/mozilla-central/rev/94cd140316fb
(Assignee)

Comment 48

8 years ago
Thanks for the quick reviews and the pushes, and sorry for the bustage.
Flags: wanted-firefox3.6?
(Assignee)

Comment 49

8 years ago
Might want to announce this in the release notes since it's hidden in the customize dialog by default.
Keywords: relnote, user-doc-needed

Updated

8 years ago
Blocks: 507861
(Assignee)

Updated

8 years ago
Blocks: 505699
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
Status: RESOLVED → VERIFIED
Keywords: verified1.9.2
Removing relnote; called out as a new feature.
Keywords: relnote
did we have Mac fullscreen before this?

Should we also be calling out that we have mac fullscreen support now?
(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.
(Assignee)

Comment 54

8 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.
user-doc-complete
https://support.mozilla.com/en-US/kb/Navigation+Toolbar+items
Keywords: user-doc-needed → user-doc-complete
You need to log in before you can comment on or make changes to this bug.