Last Comment Bug 585370 - Implement the Firefox button on Linux
: Implement the Firefox button on Linux
Status: VERIFIED FIXED
[see comment 69]
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All Linux
: -- normal with 17 votes (vote)
: Firefox 4.0b8
Assigned To: Bill Gianopoulos [:WG9s]
:
Mentors:
Depends on: 613911 645700 593126 604650 604651 608522 608717 613802 613872 621549 626055 648848
Blocks: 552302 FirefoxButton 572482 580741 598293 608555
  Show dependency treegraph
 
Reported: 2010-08-07 14:50 PDT by Bill Gianopoulos [:WG9s]
Modified: 2011-05-19 11:23 PDT (History)
57 users (show)
mozillamarcia.knous: in‑litmus?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
possible solution (2.72 KB, patch)
2010-08-11 17:17 PDT, Bill Gianopoulos [:WG9s]
dao+bmo: review-
Details | Diff | Review
theme code for application button with tabs-on-top (1.17 KB, patch)
2010-08-18 12:03 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
theme code for application button with tabs-on-top v2 (1.22 KB, patch)
2010-08-18 13:42 PDT, Bill Gianopoulos [:WG9s]
dao+bmo: review-
Details | Diff | Review
suggestion from comment 51 (2.66 KB, patch)
2010-08-21 07:52 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
Chrome vs Firefox screenshot (74.48 KB, image/png)
2010-09-05 02:14 PDT, Michael Monreal [:monreal]
no flags Details
My idea of button with tabs-on-top (24.07 KB, image/png)
2010-09-08 14:43 PDT, Bill Gianopoulos [:WG9s]
no flags Details
Just the l10n piece of this (951 bytes, patch)
2010-09-09 16:01 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
l10n patch required for tooltip on toolbar button (checked in) (1006 bytes, patch)
2010-09-18 13:10 PDT, Bill Gianopoulos [:WG9s]
dao+bmo: review+
dietrich: approval2.0+
Details | Diff | Review
Implement applications menu as a toolbar button (50.36 KB, patch)
2010-09-22 17:32 PDT, Bill Gianopoulos [:WG9s]
dao+bmo: review-
Details | Diff | Review
Implement menubar autohide under Linux (1.61 KB, patch)
2010-09-22 17:57 PDT, Bill Gianopoulos [:WG9s]
dao+bmo: review-
Details | Diff | Review
screenshot showing button on the tabbar (14.94 KB, image/png)
2010-09-22 18:05 PDT, Bill Gianopoulos [:WG9s]
faaborg: ui‑review-
Details
screenshot showing toolbar in customize toolbar window (36.25 KB, image/png)
2010-09-22 18:09 PDT, Bill Gianopoulos [:WG9s]
no flags Details
screenshot with menu open (875.16 KB, image/png)
2010-09-22 18:27 PDT, Bill Gianopoulos [:WG9s]
no flags Details
screenshot with menu open (51.85 KB, image/png)
2010-09-22 18:53 PDT, Bill Gianopoulos [:WG9s]
no flags Details
screenshot showing button tooltip (9.62 KB, image/png)
2010-09-22 19:01 PDT, Bill Gianopoulos [:WG9s]
no flags Details
screenshot using bookmarkmenu.png icon (6.46 KB, image/png)
2010-09-25 14:44 PDT, Bill Gianopoulos [:WG9s]
no flags Details
Current WIP patch to implement Firefox menu as a toolbar item (55.98 KB, patch)
2010-09-25 15:18 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
Screenshot showing icon used in WIP patch (7.08 KB, image/png)
2010-09-25 15:36 PDT, Bill Gianopoulos [:WG9s]
no flags Details
WIP patch to implement Firefox menu as a toolbar item-fixes dropmarker (56.15 KB, patch)
2010-09-28 03:10 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
WIP patch to implement Firefox menu as a toolbar item-almost there! (56.70 KB, patch)
2010-09-30 06:39 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
screenshot showing new icon (8.84 KB, image/png)
2010-09-30 06:46 PDT, Bill Gianopoulos [:WG9s]
no flags Details
implement Firefox menu as a non (re)movable toolbar item (48.79 KB, patch)
2010-09-30 13:08 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
implement Firefox menu as a non (re)movable toolbar item v2 (48.83 KB, patch)
2010-09-30 13:57 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
appmenu.png (430 bytes, image/png)
2010-10-01 03:10 PDT, Bill Gianopoulos [:WG9s]
no flags Details
comment 189 approach 1 (49.71 KB, patch)
2010-10-02 12:17 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
comment 189 approach 2 (49.33 KB, patch)
2010-10-02 12:18 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
Arrow close to a menu entry is not linked to the corresponding entry (95.27 KB, image/png)
2010-10-06 11:05 PDT, antistress
no flags Details
patch updated to tip - 2010/10/07 (50.70 KB, patch)
2010-10-07 19:04 PDT, Bill Gianopoulos [:WG9s]
dao+bmo: review-
Details | Diff | Review
addresses review comments (50.51 KB, patch)
2010-10-09 22:09 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
addresses review comments (50.52 KB, patch)
2010-10-10 06:23 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
patch v5 (51.14 KB, patch)
2010-10-11 18:46 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
patch v6 (51.59 KB, patch)
2010-10-12 21:01 PDT, Bill Gianopoulos [:WG9s]
dao+bmo: review+
Details | Diff | Review
patch v7 - addresses Michael Monreal's issues (51.76 KB, patch)
2010-10-13 08:27 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
v7 screenshot (26.22 KB, image/png)
2010-10-13 08:51 PDT, Michael Monreal [:monreal]
no flags Details
patch v8 (51.69 KB, patch)
2010-10-13 11:16 PDT, Bill Gianopoulos [:WG9s]
dao+bmo: review+
faaborg: ui‑review-
Details | Diff | Review
screenshot showing application menu in open state (51.17 KB, image/png)
2010-10-18 15:48 PDT, Bill Gianopoulos [:WG9s]
no flags Details
patch v9-fix spacing of Secondary pane menu items (51.74 KB, patch)
2010-10-19 20:55 PDT, Bill Gianopoulos [:WG9s]
wgianopoulos: review+
faaborg: ui‑review+
Details | Diff | Review
screenshot with menu open - patch v9 (27.48 KB, image/png)
2010-10-19 20:56 PDT, Bill Gianopoulos [:WG9s]
no flags Details
screenshot with menu open - patch v9 - inv theme (26.52 KB, image/png)
2010-10-19 20:57 PDT, Bill Gianopoulos [:WG9s]
no flags Details
screenshot with menu open - patch v9 - with windows color for secondary pane (39.16 KB, image/png)
2010-10-19 21:06 PDT, Bill Gianopoulos [:WG9s]
no flags Details
screenshot with menu open - patch v9 - with windows color inv theme (33.87 KB, image/png)
2010-10-19 21:10 PDT, Bill Gianopoulos [:WG9s]
no flags Details
patch v10-use text label with brandShortName instead of icon (50.34 KB, patch)
2010-10-24 17:03 PDT, Bill Gianopoulos [:WG9s]
dao+bmo: review-
wgianopoulos: ui‑review+
Details | Diff | Review
screenshot showing new button on tabbar - patch v10 (14.90 KB, image/png)
2010-10-24 17:13 PDT, Bill Gianopoulos [:WG9s]
no flags Details
screenshot of themed button (15.56 KB, image/png)
2010-10-24 18:13 PDT, Bill Gianopoulos [:WG9s]
no flags Details
patch v11-including my attempt to theme the button (50.50 KB, patch)
2010-10-24 18:52 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
patch v12-fix button height and other review issues (300.22 KB, patch)
2010-10-26 16:26 PDT, Bill Gianopoulos [:WG9s]
wgianopoulos: ui‑review+
Details | Diff | Review
patch v12a-fix button height and other review issues (52.05 KB, patch)
2010-10-27 03:05 PDT, Bill Gianopoulos [:WG9s]
dao+bmo: review-
wgianopoulos: ui‑review+
Details | Diff | Review
patch v13-fix review issues (52.05 KB, patch)
2010-10-29 06:37 PDT, Bill Gianopoulos [:WG9s]
dao+bmo: review+
wgianopoulos: ui‑review+
Details | Diff | Review
interdiff v12a to v13 (1.22 KB, patch)
2010-10-29 06:44 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
patch v14-remove code already landed and that split off with bug 608717 (2.51 KB, patch)
2010-11-01 16:24 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
patch v14-remove code already landed and that split off with bug 608717 (13.15 KB, patch)
2010-11-01 16:35 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
patch v15-remove code already landed and that split off with bug 608717 (13.15 KB, patch)
2010-11-01 19:22 PDT, Bill Gianopoulos [:WG9s]
no flags Details | Diff | Review
patch v16-update to merge with latest bug 608717 patch (13.04 KB, patch)
2010-11-01 20:19 PDT, Bill Gianopoulos [:WG9s]
wgianopoulos: ui‑review+
Details | Diff | Review
patch v17;ui-review=faaborg (13.12 KB, patch)
2010-11-02 18:26 PDT, Bill Gianopoulos [:WG9s]
dao+bmo: review+
Details | Diff | Review
patch v18;r=dao,ui-review=faaborg (13.24 KB, patch)
2010-11-03 07:01 PDT, Bill Gianopoulos [:WG9s]
wgianopoulos: review+
wgianopoulos: ui‑review+
Details | Diff | Review
patch, updated to tip (11.17 KB, patch)
2010-11-12 02:01 PST, Dão Gottwald [:dao]
dolske: approval2.0+
Details | Diff | Review
screenshot with menu open - patch v19 (28.91 KB, image/png)
2010-11-20 03:44 PST, Bill Gianopoulos [:WG9s]
faaborg: ui‑review+
Details

Description Bill Gianopoulos [:WG9s] 2010-08-07 14:50:12 PDT
It seems that the entire look and feel of Firefox is being changed under Windows for Firefox 4, but Linux will be left with the Firefox 3 look.

The reason for this seems to be that there seems to be some unnecessary tie-in in developers minds between implementing the Firefox button and enabling drawing in the titlebar.

It seems to me that implementing making the menubar be hide-able and having the Firefox button at the left of the tabbar as Firefox Beta3 is going to work under Windows/XP should be easy to implement under Linux to give people the option to use the old UI or chose a more Firefox 4 Windows-like UI by choosing to hide the menubar under Linux.
Comment 1 Dave Garrett 2010-08-07 15:51:51 PDT
It would also be great to be able to hide (not draw in) the titlebar when maximized to get the full intended effect when maximized, if not windowed.
Comment 2 Mike Beltzner [:beltzner, not reading bugmail] 2010-08-10 14:17:52 PDT
Stephen has more information here, but the Linux distributions suggested that we not do this due to upcoming platform changes on their side.
Comment 3 Dave Garrett 2010-08-10 14:26:28 PDT
I would suggest that Firefox builds from Mozilla simply aim to be as updated and streamlined as possible, and if any Linux distributions wish to do something differently they can just customize their build as desired. In other words, don't penalize everyone else because of what any individual distos may want. It's not like we can get everyone to agree on everything.
Comment 4 Stephen Horlander [:shorlander] 2010-08-10 14:34:05 PDT
(In reply to comment #3)
> I would suggest that Firefox builds from Mozilla simply aim to be as updated
> and streamlined as possible, and if any Linux distributions wish to do
> something differently they can just customize their build as desired. In other
> words, don't penalize everyone else because of what any individual distos may
> want. It's not like we can get everyone to agree on everything.

The ability to natively draw widgets in the titelbar should come with gtk3. This will give us the ability to do what we want the right way. Doing this entirely on the Firefox side would be a lot of work for a possibly degraded experience that would soon become redundant. 

I think what is work doing is implementing the Firefox button on Linux as an option in much the same way it is presently working on Windows XP.
Comment 5 Bill Gianopoulos [:WG9s] 2010-08-10 16:47:40 PDT
(In reply to comment #4)
> The ability to natively draw widgets in the titelbar should come with gtk3.
> This will give us the ability to do what we want the right way. Doing this
> entirely on the Firefox side would be a lot of work for a possibly degraded
> experience that would soon become redundant. 
> 
> I think what is work doing is implementing the Firefox button on Linux as an
> option in much the same way it is presently working on Windows XP.

I am wondering if there is objection from some of the distros to trying to make the ALT key do a function when pressed alone?  After all it is supposed to be a modifier key which by definition performs no action on its own but merely modifies some other keyboard or mouse action that is performed in conjunction with pressing the key.

The reason I ask is that I have a patch that I have been running on my own system a few days that gets around the issue of the ALT key not being easily made to make the Menubar appear under Linus by using a trick of adding an empty menu to the menubar with an access key of "M" so that you make the menubar appear by pressing ALT+M.

What I like about this approach is:

1.  It is a minimal patch
2.  It does not pervert the definition of ALT as a modifier key.

What I don't like is:

1.  It is not exactly the way it works under windows.
2.  Although ALT+M makes sense in an en-US build, I have no idea about other localizations.  IT might not be particularly mnemonic, or worse yet might already be used as an access key for one of the other menubar choices.  I think perhaps the key sequence used here should be more universal.
Comment 6 Mike Beltzner [:beltzner, not reading bugmail] 2010-08-10 17:31:42 PDT
(In reply to comment #5)

Not sure that this is on-topic for this bug.
Comment 7 Bill Gianopoulos [:WG9s] 2010-08-10 18:03:49 PDT
(In reply to comment #6)
> (In reply to comment #5)
> 
> Not sure that this is on-topic for this bug.

Well, the current situation is that there is no code that needs to be written to implement the Firefox Button under Linux (except that it is currently entirely unthemed and is really just a text button with a dropmarker).  All the code exists in the tree but is non-functional because the menubar autohide function has been disabled under Linux becuase of the issue of not being able to do the unhide with the press of the ALT key alone.
Comment 8 Mike Gualtieri 2010-08-11 06:43:58 PDT
(In reply to comment #4)
> The ability to natively draw widgets in the titelbar should come with gtk3.
> This will give us the ability to do what we want the right way. Doing this
> entirely on the Firefox side would be a lot of work for a possibly degraded
> experience that would soon become redundant. 

As far as I can tell, GTK+3 won't be released for quite some time.  It appears that the API is still under heavy development, and the latest Gnome snapshot (2.31.6) just released a few days ago is still built on GTK+2.  I think it would be a mistake to wait on GTK3, unless we are talking about Firefox 5 or 6.
Comment 9 Mike Gualtieri 2010-08-11 06:59:12 PDT
Sorry to post twice in a row, but I wanted to make a suggestion.

Why not consider moving the firefox button into the navigation bar / tab bar bookmarks bar?  In addition to API compatibility problems, drawing in the title bar area might be a larger issue for some non-GTK based Window managers.
Comment 10 Dave Garrett 2010-08-11 07:12:59 PDT
In reply to comment 7: To repeat something from bug 552302 here: addons have existed for quite a while that can hide/show the menu bar under Linux just fine. There's no reason to not add the ability by default to match other platforms. Again, if a distro disagrees let them deal with their build themselves.

In reply to comment 9: comment 0 already suggests the tab bar, which is done on Windows XP and is the most logical and simple thing to do here.
Comment 11 Bill Gianopoulos [:WG9s] 2010-08-11 08:45:33 PDT
(In reply to comment #8)
> As far as I can tell, GTK+3 won't be released for quite some time.  It appears
> that the API is still under heavy development, and the latest Gnome snapshot
> (2.31.6) just released a few days ago is still built on GTK+2.  I think it
> would be a mistake to wait on GTK3, unless we are talking about Firefox 5 or 6.

I believe the concern here is not that we should wait for GTK3 before doing anything, just try not to do a kludge that works with GTK2 that is likely to make Firefox incompatible with GTK3.

I don't think that having everyone working on GTK3 development switch to no longer using Firefox as their default browser because it is incompatible, is a desirable outcome here.
Comment 12 Dave Garrett 2010-08-11 10:03:32 PDT
One thing that has yet to be mentioned: If the Firefox button isn't implemented under Linux, this effectively drops Linux support for any addons that intend to add entries to the menu. No button -> errors overlaying + developer who doesn't touch Linux = no Linux support => not really fully supported OS anymore.
Comment 13 Dave Garrett 2010-08-11 10:04:43 PDT
Requesting blocking for at least optional support as per Stephen in comment 4.
Comment 14 Mike Beltzner [:beltzner, not reading bugmail] 2010-08-11 10:33:24 PDT
(In reply to comment #12)
> One thing that has yet to be mentioned: If the Firefox button isn't implemented
> under Linux, this effectively drops Linux support for any addons that intend to
> add entries to the menu. No button -> errors overlaying + developer who doesn't
> touch Linux = no Linux support => not really fully supported OS anymore.

We'll have no Firefox button on WinXP or Win2K, either. The idea is that add-on authors will overlay a hook which puts the menuItems in the appopriate place, I believe.
Comment 15 Dave Garrett 2010-08-11 10:46:27 PDT
(In reply to comment #14)
> We'll have no Firefox button on WinXP or Win2K, either.

WinXP has the button, it's just hidden by default. (I'm not sure about Win2K, but I think it's the same as WinXP; correct me if not) Linux doesn't have it at all.
Comment 16 Michael Monreal [:monreal] 2010-08-11 11:13:35 PDT
(In reply to comment #14)
> The idea is that add-on authors will overlay a hook which puts the 
> menuItems in the appopriate place, I believe.

Is that possible atm? It would be great if we could provide a semi-official extension for this. Just a toolbar button which calls up the menu. So, all we really need is a way to access this menu from extensions.
Comment 17 Bill Gianopoulos [:WG9s] 2010-08-11 17:00:56 PDT
(In reply to comment #16)
> (In reply to comment #14)
> > The idea is that add-on authors will overlay a hook which puts the 
> > menuItems in the appopriate place, I believe.
> 
> Is that possible atm? It would be great if we could provide a semi-official
> extension for this. Just a toolbar button which calls up the menu. So, all we
> really need is a way to access this menu from extensions.

I have an extension that I support that adds menu item to either the classic file menu or the Firefox menu or both depending on shat is available.  It is relatively easy to do.
Comment 18 Bill Gianopoulos [:WG9s] 2010-08-11 17:17:52 PDT
Created attachment 465030 [details] [diff] [review]
possible solution

This is the patch I alluded to in comment #5.

I suspect that due to the lack of anyone who says there is this code that works perfectly had provided a reviewable patch to date.  This is the best path going forward,.  If someone comes along with an approved patch to allow the ALT key alone to un-hide the menubar then that is great, but since that does not appear to be forthcoming, I am hoping that perhaps this will be accepted as the best way to get some support for the Firefox button under Linux in time for the Firefox 4 release.

The patch has 3 parts.

1.  The code in browser/base/content/browser-menubar.inc and browser/locales/en-US/chrome/browser/browser.dtd implements the ATL-M to un-hide the Menubar.

2.  The code in toolkit/content/xul.css reverts the code from bug 498628 which disabled autohide under Linux because there was no key combination to unhide the menubar without opening a menu.   This limitation is fixed by part 1 of this patch.

3.  The code in browser/base/Makefile.in defines MENUBAR_CAN_AUTOHIDE for Linux which enables the Firefox button menu.

Builds containing this patch are available at my usual test location:

http://www.wg9s.com/mozilla/firefox/
Comment 19 Dave Garrett 2010-08-11 18:05:09 PDT
Alt-M is not a good way to go, I think.

Again, the Hide Menubar extension shows with just a press of alt and works just fine. I'm using it right now, and have been since the Personal Menu extension switched to using it instead of it doing the exact same thing itself for a very long time. There have been extensions that have allowed hiding of the menu bar and reshowing it by pressing alt for years, and Linux is supported just fine. I see no reason to not just use alt like elsewhere, yanking any code or ideas from one of the many extensions that have implemented this if needed.
Comment 20 Bill Gianopoulos [:WG9s] 2010-08-11 18:08:50 PDT
(In reply to comment #19)
> Alt-M is not a good way to go, I think.
> 
> Again, the Hide Menubar extension shows with just a press of alt and works just
> fine. I'm using it right now, and have been since the Personal Menu extension
> switched to using it instead of it doing the exact same thing itself for a very
> long time. There have been extensions that have allowed hiding of the menu bar
> and reshowing it by pressing alt for years, and Linux is supported just fine. I
> see no reason to not just use alt like elsewhere, yanking any code or ideas
> from one of the many extensions that have implemented this if needed.

The hide menubar extension works by putting a lot of raw event handling code in the XUL level that is required for Linux only.  Code at the XUL level should be platform agnostic.  Therefore, this approach is really a non-starter as a solution.

Linux only code would really need to be in the back-end someplace and not a kludge adde at the XUL layer.  Just my opinion.  I could be wrong.
Comment 21 Dave Garrett 2010-08-11 18:26:23 PDT
(In reply to comment #20)
> The hide menubar extension works by putting a lot of raw event handling code in
> the XUL level that is required for Linux only.  Code at the XUL level should be
> platform agnostic.  Therefore, this approach is really a non-starter as a
> solution.
> 
> Linux only code would really need to be in the back-end someplace and not a
> kludge adde at the XUL layer.  Just my opinion.  I could be wrong.

Then either some other better implementation can be done or the same hacks can be done better. One way or another, it's quite doable. We may have to simply accept that the Right Way™ may not be an option and the hacky way is best for now.

(In reply to comment #17)
> I have an extension that I support that adds menu item to either the classic
> file menu or the Firefox menu or both depending on shat is available.  It is
> relatively easy to do.

This requires thought; some developers don't make with the thinking as much as they should. The most common way I've seen people not support Linux is just because they never thought to. If everything the developer has seen has the Firefox button, I see no reason why some wouldn't rely on its existence. (or shouldn't for that matter) One way or another, any big platform inconsistencies like this are problems for extension and also theme support.
Comment 22 Bill Gianopoulos [:WG9s] 2010-08-11 18:37:24 PDT
Well, one of the issues is also that one of the major complaints about Firefox is that compared to chrome, it is a bit slow and klunky.  Trying to process more raw keyboard events at the XUL level, as these extensions do, is probably not step in the direction toward making things faster.
Comment 23 Michael Monreal [:monreal] 2010-08-12 01:40:26 PDT
Bill, I just tested your latest build and while it does show a firefox button, it does so in a separate row and still shows the old menubar.

The only real place for the firefox button to live would be in front of the first tab or as configurable toolbar button.
Comment 24 Bill Gianopoulos [:WG9s] 2010-08-12 02:59:37 PDT
(In reply to comment #23)
> Bill, I just tested your latest build and while it does show a firefox button,
> it does so in a separate row and still shows the old menubar.
> 
> The only real place for the firefox button to live would be in front of the
> first tab or as configurable toolbar button.

Yes, my plan was that if the approach in this patch were accepted then I would get it to work similar to the way it does under windows XP in the beta3 build.  The button would be to the left of the tabs if tabs-on-top is selected and on a separate row otherwise.
Comment 25 Michael Monreal [:monreal] 2010-08-12 03:21:18 PDT
(In reply to comment #24)
> The button would be to the left of the tabs if tabs-on-top is selected and on a
> separate row otherwise.

We really need some UI decisions here... but having the button on a separate row doesn't make a lot of sense, after all the point is to reduce chrome.
Comment 26 Alex Limi (:limi) — Firefox UX Team 2010-08-12 17:37:24 PDT
On Linux, it's supposed to be on the same line as the tab bar, but not in the window chrome for technical reasons (until GNOME adds this capability), so like this:

===== Window title ======================[_][x]
[Firefox v] [ tab 1 ] [ tab 2 ][+] 
[<] [>]  [http://example.com]
Comment 27 Pascal Chevrel:pascalc 2010-08-12 17:52:59 PDT
@limi, what happens if the option "Always show the tab bar" is not ticked for Linux?
Comment 28 Michael Monreal [:monreal] 2010-08-13 00:44:50 PDT
(In reply to comment #27)
> @limi, what happens if the option "Always show the tab bar" is not ticked for
> Linux?

I would say we don't care and just display

===== Window title ======================[_][x]
[<] [>]  [http://example.com]
[Firefox v] [ tab 1 ] [ tab 2 ][+]

Easiest to implement and probably not widely used.

NOTE: there is a problem with tabs-on-top and tabs dragging when there is not menu. Try it. I think we need to increase top padding in that case and also use a smaller tab drop indicator.
Comment 29 Bill Gianopoulos [:WG9s] 2010-08-13 06:03:37 PDT
I looked into how the hack for this on Windows XP works in the beta, and unfortunately it is just that.  It is roughly the equivalent of this code I just added to my userChrome.css file.

#navigator-toolbox[tabsontop="true"] > #toolbar-menubar[autohide="true"] ~ #TabsToolbar:not([inFullscreen]) {
  -moz-padding-start: 7.5em !important;
  margin-top: -1.7em !important;
}

As you can see the spacing here is dependent on the length of brandShortname and fonts being used.
Comment 30 Bill Gianopoulos [:WG9s] 2010-08-13 16:26:42 PDT
I should have mentioned that I shared that css just to provide kind of a starting point to talk about the possible layout.

It really does not work correctly.  The button ends up not being clickable except along the top and at the area where the brandShortName appeared, the drop marker and right margin as well as the area under the label are not clickable.

It also does not work correctly with tabs-on-top in full screen mode.  The label appears over the navigation bar.

It also does not display correctly when in toolbar customization mode if you have the menubar hidden and tabs-on-top selected.
Comment 31 Bill Gianopoulos [:WG9s] 2010-08-15 12:14:01 PDT
A better UserChrome.css hack:

#navigator-toolbox[tabsontop="true"] > #toolbar-menubar[autohide="true"] ~ #TabsToolbar:not([inFullscreen]) {
  margin-left: 7.3em !important;
  margin-top: -2.375em !important;
}
#titlebar-spacer, #titlebar-buttonbox {
  display: none !important;
}

This works correctly with the tabcandy and display all tabs icon and tweaks the margins to make things actually align correctly.  It still is a bad hack that depends on the length of brandShortName and also does not have all of the appbutton being click-able as it should be.
Comment 32 Bill Gianopoulos [:WG9s] 2010-08-15 12:22:28 PDT
I should have mentioned that my builds at http://www.wg9s.com/mozilla/firefox/  now include both the ATL+M hack, and this CSS hack, built into the default Linux theme.
Comment 33 Bill Gianopoulos [:WG9s] 2010-08-15 13:01:42 PDT
Comment on attachment 465030 [details] [diff] [review]
possible solution

Asking for review on this.  Not because I think this is the absolute correct solution, but because with the current situation of the application button being dependent on autohide, and autohide being disabled because of a lack of a way under Linux to un0autohide without opening a men u pop-up, this is a way to permit people to work on how to do the theme work on the application button under Linux in parallel with someone smarter than me figuring out a way to get this to work with just the ALT key.

I think this is a way to move forward here by providing a method to un-hide under Linux that does not open a pop-up, although it is via a different key-sequence than what works under Windows.
Comment 34 Dave Garrett 2010-08-15 13:08:05 PDT
There is a third option, you know: no auto-unhide. Just let the menu bar be toggled via the menu and not by alt or a hotkey. Just put the toggle menu entry in the Firefox button menu somewhere and have that keyboard accessible as would be the menu entry in the regular menu.
Comment 35 Bill Gianopoulos [:WG9s] 2010-08-15 13:18:49 PDT
(In reply to comment #34)
> There is a third option, you know: no auto-unhide. Just let the menu bar be
> toggled via the menu and not by alt or a hotkey. Just put the toggle menu entry
> in the Firefox button menu somewhere and have that keyboard accessible as would
> be the menu entry in the regular menu.

Well, you know, that is kind of my feeling as well.  The only reason we currently need the hot-key unhide is because the check for updates is not an option under the application button and because of extensions that need to make the options they add under tools in the classic menu be somehow available under the new applications menu.
Comment 36 Bill Gianopoulos [:WG9s] 2010-08-15 13:30:07 PDT
So having the applications button appear should be dependent upon the menubar not being displayed.  Rather than on it's ability to autohide, as it is now.
Comment 37 Dão Gottwald [:dao] 2010-08-16 12:04:46 PDT
(In reply to comment #34)
> There is a third option, you know: no auto-unhide. Just let the menu bar be
> toggled via the menu and not by alt or a hotkey. Just put the toggle menu entry
> in the Firefox button menu somewhere and have that keyboard accessible as would
> be the menu entry in the regular menu.

See bug 575279 comment 6...
Also, somebody would actually have to make all App button menu contents keyboard accessible, not just the button itself.
Comment 38 Bill Gianopoulos [:WG9s] 2010-08-18 12:03:08 PDT
Created attachment 467085 [details] [diff] [review]
theme code for application button with tabs-on-top

This implements the same theme code as in Winstripe for the Windows/XP aero basic case.
Comment 39 Bill Gianopoulos [:WG9s] 2010-08-18 13:42:07 PDT
Created attachment 467143 [details] [diff] [review]
theme code for application button with tabs-on-top v2
Comment 40 Bill Gianopoulos [:WG9s] 2010-08-19 10:21:54 PDT
Comment on attachment 465030 [details] [diff] [review]
possible solution

Realized Dao did not seem to be a peer for this module. Requesting a review from Gavin.
Comment 41 Bill Gianopoulos [:WG9s] 2010-08-19 10:23:11 PDT
Comment on attachment 467143 [details] [diff] [review]
theme code for application button with tabs-on-top v2

This is essentially the same fix checked in for Windows in bug 574434.
Comment 42 :Gavin Sharp [email: gavin@gavinsharp.com] 2010-08-19 10:26:24 PDT
Dao can review this (and is probably a better choice).
Comment 43 Bill Gianopoulos [:WG9s] 2010-08-19 10:47:37 PDT
(In reply to comment #42)
> Dao can review this (and is probably a better choice).

OK.  That is what I thought originally as much of the first patch is just re-instating code he had removed in the first place because he did not like the way hiding the menubar worked under Linux.
Comment 44 Dão Gottwald [:dao] 2010-08-20 06:05:27 PDT
Comment on attachment 465030 [details] [diff] [review]
possible solution

>--- a/browser/base/content/browser-menubar.inc
>+++ b/browser/base/content/browser-menubar.inc
>@@ -683,9 +683,14 @@
> #endif
>               </menupopup>
>             </menu>
> 
> #ifdef XP_MACOSX
>           <menu id="windowMenu" />
> #endif
>           <menu id="helpMenu" />
>+#ifdef MOZ_WIDGET_GTK2
>+          <menu id="menu-menu" label=" "
>+            accesskey="&menuBar.accesskey;">
>+          </menu>
>+#endif
>         </menubar>

This seems entirely non-discoverable. At that point we could as well have no shortcut for showing the menu bar without opening a menu -- which might be okay as long as we don't hide the menu bar by default.
Comment 45 Dão Gottwald [:dao] 2010-08-20 06:08:41 PDT
Comment on attachment 467143 [details] [diff] [review]
theme code for application button with tabs-on-top v2

>--- a/browser/themes/gnomestripe/browser/browser.css
>+++ b/browser/themes/gnomestripe/browser/browser.css
>@@ -1190,16 +1190,35 @@ statusbarpanel#statusbar-display {
> /* Tabs */
> #TabsToolbar {
>   -moz-appearance: none;
>   min-height: 0;
>   padding: 0;
>   -moz-box-shadow: ThreeDShadow 0 -1px inset;
> }
> 
>+/* XXX: stop-gap until the Application button can be drawn in the title bar */

Unlike on Windows, we're not going to be able to do this on Linux soon enough. It's a hack and only acceptable on Windows because we know it's going away. We should just make it a toolbar item on Linux.
Comment 46 Bill Gianopoulos [:WG9s] 2010-08-20 06:40:38 PDT
(In reply to comment #44)
> This seems entirely non-discoverable. At that point we could as well have no
> shortcut for showing the menu bar without opening a menu -- which might be okay
> as long as we don't hide the menu bar by default.

I kind of thought it a bit kludgy myself.  The main reason I offered it was if there was a plan to get this to work with just the ALT button eventually this could be landed first to give theme people a chance to work on the theme in relation to the app button using this code.

I still think the entire idea of the application button being dependent on the ability to hide the menubar is just wrong.

What about a preference for menu styhle with 3 values:

Application button
Classic manubar
both

With the both choice only being offered on platfroms where the auto-hide is supported?
Comment 47 Bill Gianopoulos [:WG9s] 2010-08-20 06:43:11 PDT
(In reply to comment #45)
> Unlike on Windows, we're not going to be able to do this on Linux soon enough.
> It's a hack and only acceptable on Windows because we know it's going away. We
> should just make it a toolbar item on Linux.

I love the idea of this being a toolbar button, so presumably this could be placed where desired, including on the left side of the bookmarks toolbar.
Comment 48 Michael Monreal [:monreal] 2010-08-20 06:46:07 PDT
(In reply to comment #45)
>> We should just make it a toolbar item on Linux

There should really be UX input for this but personally I think showing a
toolbar item next to the bookmarks button only when the menubar is hidden would
work best and require the least amount of hacks. Making the button placable by the user is not what we want IMHO.

We could style this with some orange icon to match the firefox button on windows.

CONS:
- menu access on the right instead of the left

PROS:
- choice for the user to get rid of the menu bar
- easy to implement
- losing about 30 pixels of the awesomebar is cheap compared to losing quite
some space in the tab bar
- similar to google chrome
- similar to how many GNOME 3 apps will handle this (I can provide current mockups if anyone is interested)

In the end let's not forget that this will not be the default
Comment 49 Mike Gualtieri 2010-08-20 07:18:10 PDT
Placement as a toolbar icon is by far the best proposed solution, IMHO.  This likely wins in the long-term too.  Even if GTK-3 allows the placement of such buttons in the titlebar, that doesn't mean that all window managers will support this -- especially the more obscure WM's.
Comment 50 Bill Gianopoulos [:WG9s] 2010-08-20 07:24:59 PDT
(In reply to comment #48)
> (In reply to comment #45)
> >> We should just make it a toolbar item on Linux
> 
> There should really be UX input for this but personally I think showing a
> toolbar item next to the bookmarks button only when the menubar is hidden would
> work best and require the least amount of hacks. Making the button placable by
> the user is not what we want IMHO.
> 
> We could style this with some orange icon to match the firefox button on
> windows.
> 
> CONS:
> - menu access on the right instead of the left
> 
> PROS:
> - choice for the user to get rid of the menu bar
> - easy to implement
> - losing about 30 pixels of the awesomebar is cheap compared to losing quite
> some space in the tab bar
> - similar to google chrome
> - similar to how many GNOME 3 apps will handle this (I can provide current
> mockups if anyone is interested)
> 
> In the end let's not forget that this will not be the default

(In reply to comment #48)
> (In reply to comment #45)
> >> We should just make it a toolbar item on Linux
> 
> There should really be UX input for this but personally I think showing a
> toolbar item next to the bookmarks button only when the menubar is hidden would
> work best and require the least amount of hacks. Making the button placable by
> the user is not what we want IMHO.
> 
> We could style this with some orange icon to match the firefox button on
> windows.
> 
> CONS:
> - menu access on the right instead of the left
> 
> PROS:
> - choice for the user to get rid of the menu bar
> - easy to implement
> - losing about 30 pixels of the awesomebar is cheap compared to losing quite
> some space in the tab bar
> - similar to google chrome
> - similar to how many GNOME 3 apps will handle this (I can provide current
> mockups if anyone is interested)
> 
> In the end let's not forget that this will not be the default

The whole point of making it placeable by the user is that the user can make the choice on the trade-off between losing 30 pixels on the awesomebar and "losing quite some space in the tab bar".

Users who do not typically have more that 4 or 5 tabs open, would probably prefer this on the tabbar.
Comment 51 Bill Gianopoulos [:WG9s] 2010-08-20 07:31:32 PDT
(In reply to comment #48)
> - losing about 30 pixels of the awesomebar is cheap compared to losing quite
> some space in the tab bar

Oh I really don't like the way this is done under windows.  I think this belongs on the left ot the tabs with tabs-on top and instead of being brandShortname with a dropmarker, it should merely be the Firefox icon, which would not require a great deal of horizontal real-estate.
Comment 52 Mike Gualtieri 2010-08-20 07:33:14 PDT
> The whole point of making it placeable by the user is that the user can make
> the choice on the trade-off between losing 30 pixels on the awesomebar and
> "losing quite some space in the tab bar".
> 
> Users who do not typically have more that 4 or 5 tabs open, would probably
> prefer this on the tabbar.

This is a good point, but simplicity of development should also be considered. 
It seems to me that such functionality is already built into the toolbar, but
the tabbar doesn't really support adding buttons -- at least not in Firefox
3.6.
Comment 53 Bill Gianopoulos [:WG9s] 2010-08-20 07:37:13 PDT
(In reply to comment #52)
> > The whole point of making it placeable by the user is that the user can make
> > the choice on the trade-off between losing 30 pixels on the awesomebar and
> > "losing quite some space in the tab bar".
> > 
> > Users who do not typically have more that 4 or 5 tabs open, would probably
> > prefer this on the tabbar.
> 
> This is a good point, but simplicity of development should also be considered. 
> It seems to me that such functionality is already built into the toolbar, but
> the tabbar doesn't really support adding buttons -- at least not in Firefox
> 3.6.

Sigh!  You are correct.  I should have checked that first.
Comment 54 Michael Monreal [:monreal] 2010-08-20 07:40:06 PDT
(In reply to comment #52)
> It seems to me that such functionality is already built into the toolbar, but
> the tabbar doesn't really support adding buttons -- at least not in Firefox
> 3.6.

That is possible right now (see new tab and tabcandy buttons... tabs basicly live in a toolbar now). But the discussion is not really going anywhere. The UX team needs to make a decision.
Comment 55 Bill Gianopoulos [:WG9s] 2010-08-21 07:52:23 PDT
Created attachment 468043 [details] [diff] [review]
suggestion from comment 51

Posting for reference only.
Comment 56 Mike Beltzner [:beltzner, not reading bugmail] 2010-08-24 08:11:48 PDT
Minusing; the UX team can renominate if they think that this blocks the release.
Comment 57 Alex Limi (:limi) — Firefox UX Team 2010-08-24 16:11:04 PDT
I personally don't think this should be blocking 4.0 if we can't make it, agreed.

As for the intended behavior: Ideally, we'd have the option to show the Firefox menu on the same level as the tab strip, as earlier specified:

===== Window title ======================[_][x]
[Firefox v] [ tab 1 ] [ tab 2 ][+] 
[<] [>]  [http://example.com]

If people turn off the tab bar, when they have added the Firefox menu button to it, it goes away with the tab bar. It should behave like (and I assume, *be*) a normal toolbar button. (not sure what the correct terminology here is).
Comment 58 sashafan2 2010-08-26 01:54:25 PDT
(In reply to comment #57)
> I personally don't think this should be blocking 4.0 if we can't make it,
> agreed.
> 
> 
I think this definitely should block the release. It'll be a shame if Linux version releases with the old interface and will lower the popularity of Firefox on Linux.
Comment 59 Alex Limi (:limi) — Firefox UX Team 2010-08-26 17:45:00 PDT
(In reply to comment #58)
> I think this definitely should block the release. It'll be a shame if Linux
> version releases with the old interface and will lower the popularity of
> Firefox on Linux.

That's not why the menu button exists; Linux doesn't have this kind of UI, whereas Windows 7 & Vista do.

Linux is like Mac OS X in this respect, where the menu bar is the established way to interact with an application. We'd like to offer the option to use the new-style menu button on platforms that don't normally use this as an option (see: OS X, XP, Linux), but it's not going to block the release in itself.
Comment 60 Alex Faaborg [:faaborg] (Firefox UX) 2010-08-26 18:56:28 PDT
>Linux doesn't have this kind of UI

Although ironically the Firefox button may have influenced some of the decisions being made for Gnome's UI, in which case we may eventually get a native control to use :) (talk about coming full circle).

But yeah, in the meantime our overall goal is to create an interface that is native and familiar given the interactive design of the surrounding operating system.
Comment 61 Bill Gianopoulos [:WG9s] 2010-08-27 10:32:28 PDT
(In reply to comment #60)
> >Linux doesn't have this kind of UI
> 
> Although ironically the Firefox button may have influenced some of the
> decisions being made for Gnome's UI, in which case we may eventually get a
> native control to use :) (talk about coming full circle).
> 
> But yeah, in the meantime our overall goal is to create an interface that is
> native and familiar given the interactive design of the surrounding operating
> system.

Well the problem with that approach is that I assume I am not the only one who develops Firefox extensions under Linux that add items to the Menu.  Without having the New Application Button menu in some way usable under Linux this severely impacts my ability to properly support my extensions.

Not having some way to have this work at least with a hidden preference is a hinderence to extension developers.
Comment 62 Dave Garrett 2010-08-30 18:17:12 PDT
When fun things like this are being considered:
https://bug592147.bugzilla.mozilla.org/attachment.cgi?id=470654

Not having the Firefox button, at least optionally, means not being real full Firefox. People will extend this. It must be in.

(In reply to comment #59)
> We'd like to offer the option to use the
> new-style menu button on platforms that don't normally use this as an option
> (see: OS X, XP, Linux), but it's not going to block the release in itself.

It will be an awful mess in addon land, both extensions and themes, if Firefox starts dropping cross-platform support in any part. Not having the button on certain platforms does just that. I don't see how this can't block.
Comment 63 Bill Gianopoulos [:WG9s] 2010-09-04 15:47:31 PDT
I obsoleted all the patches here as they no longer apply cleanly and also did not work correctly if your OS theme was a dark background with white text.

If anyone is still interested in this approach, my builds and patches are available at http://www.wg9s.com/mozilla/firefox/
Comment 64 Michael Moravec 2010-09-04 16:27:28 PDT
I agree with comment #62.

In my opinion, this issue should block release, definitely.

Waiting for GTK+ 3 is not a way IMO, ie. Debian's next release (Squeeze, frozen
now) will contain GTK+ 2 - GTK+ 3 will be (hopefully) present in next stable
release (2014/2015 - just guessing). So Firefox 4/5/6 will look like Firefox 3
until then? Strange.

I don't like platform specific UI changes, this is like saying "If you want
pretty Firefox, don't use Linux" or "Linux is not supported".
Comment 65 Bill Gianopoulos [:WG9s] 2010-09-04 17:03:03 PDT
Google chrome seems to be able to do this just fine without waiting for gtk3, so although the GTK developers might prefer not to have a lot of applications doing this in a way that might not be compatible with what they plan for GTK3, the genie is already out of the bottle.  It would seem at this point that waiting is just letting the competition get way ahead.
Comment 66 Michael Monreal [:monreal] 2010-09-05 02:14:27 PDT
Created attachment 472240 [details]
Chrome vs Firefox screenshot

Don't let us start this discussion again. Doing what Chrome does is not really an option because it doesn't integrate nicely with window management (no context menu) and also does not allow to preserve the style of window deco. See the attached screenshot, it shows a Chrome window (active) as well as a terminal window (inactive) and Firefox (active). As you see, Chrome looks totally out of place.

Doing the "implement as button and place before first tab" is still the way to go.
Comment 67 sashafan2 2010-09-05 05:55:55 PDT
What about the way it is done in Opera?
Comment 68 Dave Garrett 2010-09-05 09:46:16 PDT
Proposal to lower the chatter in here:
* Restrict this bug to the simple proposal (comment 26)
* File a new bug for debating fancier routes after that
Comment 69 Stephen Horlander [:shorlander] 2010-09-08 13:51:24 PDT
The desired approach for a Linux Firefox Button would be:

- Firefox button as a customizable toolbar item
- Menubar ON by default
- Add option to hide the menubar manually on Linux

 View > Toolbars > √ Menu Bar
                   √ Navigation Toolbar
                   √ Bookmarks Toolbar
                   --------------------
                   √ Tabs on top
                   --------------------
                     Customize…

- If the menubar is hidden manually the Firefox button appears next to the tabs by default but can be moved anywhere
- No menubar access key (the Firefox menu should be self sufficient and the alt key convention is Windows specific)
Comment 70 Dave Garrett 2010-09-08 14:06:07 PDT
(In reply to comment #69)
Sounds perfect.

Any suggestion to allow other OSes to move the Firefox button around too?
Comment 71 Bill Gianopoulos [:WG9s] 2010-09-08 14:10:33 PDT
(In reply to comment #69)
> The desired approach for a Linux Firefox Button would be:
> 
> - Firefox button as a customizable toolbar item
> - Menubar ON by default
> - Add option to hide the menubar manually on Linux
> 
>  View > Toolbars > √ Menu Bar
>                    √ Navigation Toolbar
>                    √ Bookmarks Toolbar
>                    --------------------
>                    √ Tabs on top
>                    --------------------
>                      Customize…
> 
> - If the menubar is hidden manually the Firefox button appears next to the tabs
> by default but can be moved anywhere
> - No menubar access key (the Firefox menu should be self sufficient and the alt
> key convention is Windows specific)

There are issues with this approach.  Most notably that it requires supporting the placement of toolbar icons on the bookmarks toolbar which is not currently supported and last I knew feature freeze was scheduled for Friday.

Also making this a toolbar butt that can be places anywhere via customize toolbars requires that the button have a text label that displays if the user selects display icons and text or text only.  As far as I know string freeze for such a label is also scheduled for Friday.

Toolbar icons currently have the option to display large icons or small icons bot with or without the text label and also have the option to display the text label only.

For this purpose I really don;t think any option makes sense except displaying small icon only without text.  So, there are a lot of UI issues to be determined none of which will be solved by Friday.
Comment 72 Bill Gianopoulos [:WG9s] 2010-09-08 14:12:06 PDT
(In reply to comment #70)
> (In reply to comment #69)
> Sounds perfect.
> 
> Any suggestion to allow other OSes to move the Firefox button around too?

Which other OSes are you referring to?  The only other primary supported OS is for OSX and Apple is very draconian about their HID which sort of requires the traditional menubar.
Comment 73 Stephen Horlander [:shorlander] 2010-09-08 14:23:22 PDT
(In reply to comment #71)
> There are issues with this approach.  Most notably that it requires supporting
> the placement of toolbar icons on the bookmarks toolbar which is not currently
> supported and last I knew feature freeze was scheduled for Friday.

I don't understand what you mean by this. You can currently place toolbar icons on the bookmarks toolbar.


> Also making this a toolbar butt that can be places anywhere via customize
> toolbars requires that the button have a text label that displays if the user
> selects display icons and text or text only.  As far as I know string freeze
> for such a label is also scheduled for Friday.

Wouldn't the label be whatever is being displayed on the button? "Minefield" or "Firefox". Technically it could go entirely label-less since the button is self describing.


> Toolbar icons currently have the option to display large icons or small icons
> bot with or without the text label and also have the option to display the text
> label only.

They do have that ability. I don't think this is something we should worry about though. The button should be fixed as a button with the word "Firefox" on it regardless of the state of the the other toolbar items. The only thing it probably should do is respect height of large vs. small icons.
Comment 74 Dave Garrett 2010-09-08 14:32:37 PDT
(In reply to comment #72)
> (In reply to comment #70)
> > Any suggestion to allow other OSes to move the Firefox button around too?
> 
> Which other OSes are you referring to?

I meant primarily letting the various versions of Windows move the button.

> The only other primary supported OS is for OSX and Apple is very draconian
> about their HID which sort of requires the traditional menubar.

I'm talking about an option, not anything by default. (and I don't really care about Apple's rules too much, personally ;)
Comment 75 Michael Monreal [:monreal] 2010-09-08 14:34:43 PDT
(In reply to comment #73)
> They do have that ability. I don't think this is something we should worry
> about though. The button should be fixed as a button with the word "Firefox" on
> it regardless of the state of the the other toolbar items.

+1... this is what the official beta feedback extension does btw... adds a
button which always shows the label "Feedback" (and a drop marker) and no icon
at all. Having the firefox button look and work like that would be great imho
Comment 76 Bill Gianopoulos [:WG9s] 2010-09-08 14:36:17 PDT
(In reply to comment #74)
> (In reply to comment #72)
> > (In reply to comment #70)
> > > Any suggestion to allow other OSes to move the Firefox button around too?
> > 
> > Which other OSes are you referring to?
> 
> I meant primarily letting the various versions of Windows move the button.

so, now besides asking for adding support to customize toolbar icons on the tabbar, which has never been supported before, you also want to support this on the titlebar as well?
Comment 77 Stephen Horlander [:shorlander] 2010-09-08 14:39:02 PDT
(In reply to comment #76)
> (In reply to comment #74)
> > (In reply to comment #72)
> > > (In reply to comment #70)
> > > > Any suggestion to allow other OSes to move the Firefox button around too?
> > > 
> > > Which other OSes are you referring to?
> > 
> > I meant primarily letting the various versions of Windows move the button.
> 
> so, now besides asking for adding support to customize toolbar icons on the
> tabbar, which has never been supported before, you also want to support this on
> the titlebar as well?

This seems off-topic for this bug :)
Comment 78 Bill Gianopoulos [:WG9s] 2010-09-08 14:43:39 PDT
Created attachment 473235 [details]
My idea of button with tabs-on-top

I kind of had a different idea.  This is based on comments form other Linux users in various forums.  Just use the application icon as an icon with no text on the left of the tab toolbar.  This takes up zero vertical space and a minimal amount of horizontal space which is what I gathered was what the users seemed to be asking for.
Comment 79 Dave Garrett 2010-09-08 14:49:04 PDT
(In reply to comment #77)
> (In reply to comment #76)
> > (In reply to comment #74)
> > > (In reply to comment #72)
> > > > (In reply to comment #70)
> > > > > Any suggestion to allow other OSes to move the Firefox button around too?

> This seems off-topic for this bug :)

Agreed; sorry for starting the tangent. ;)
Comment 80 Dave Garrett 2010-09-08 14:51:15 PDT
Comment on attachment 473235 [details]
My idea of button with tabs-on-top

That results in two identical icons directly next to eachother, unfortunately.
Comment 81 Michael Monreal [:monreal] 2010-09-08 14:58:38 PDT
(In reply to comment #80)
> That results in two identical icons directly next to eachother, unfortunately.

Not with any window manager :) but even the real firefox logo there wouldn't really scream "click me I am a menu button" to me.

Anyway, UX has decided - see comment #26 and comment #69 which has all the info that is needed.
Comment 82 Bill Gianopoulos [:WG9s] 2010-09-08 15:27:55 PDT
(In reply to comment #81)
> (In reply to comment #80)
> > That results in two identical icons directly next to eachother, unfortunately.
> 
> Not with any window manager :) but even the real firefox logo there wouldn't
> really scream "click me I am a menu button" to me.
> 
> Anyway, UX has decided - see comment #26 and comment #69 which has all the info
> that is needed.

Well I  will try one more time.

I can either do make it a toolbar icon that can be customized and put on any toolbar where customization is currently possible
  
  OR

I can put it on the left side of the tabbar in whatever form you want it to take.

I cannot do both because I do not have the necessary skills or knowledge of how all of this works to add a new toolbar to the customization scheme.  However, it is also my opinion that even if I did, such a patch would be rejected anyway for being too late for the feature freeze.

SO although UX may have spoken, I think we need a new answer.  If none is forthcoming then I guess I can't help here.
Comment 83 Bill Gianopoulos [:WG9s] 2010-09-08 15:39:16 PDT
(In reply to comment #81)
> (In reply to comment #80)
> > That results in two identical icons directly next to eachother, unfortunately.
> 
> Not with any window manager :) but even the real firefox logo there wouldn't
> really scream "click me I am a menu button" to me.

Well if it were really being done as a customizable toolbar icon, I woudl do this with a dropmarker ant the text albe woudl be appShortname so for test only would be a textbutton with the application name and a dropmarker and for icon only would at least have a dropmarker.

I was thinking after posting my screenshot that this really at least needs a dropmarpker.
Comment 84 Dave Garrett 2010-09-08 15:44:59 PDT
(In reply to comment #82)
> (In reply to comment #81)
> > Anyway, UX has decided - see comment #26 and comment #69 which has all the info
> > that is needed.
> 
> Well I  will try one more time.
> 
> I can either do make it a toolbar icon that can be customized and put on any
> toolbar where customization is currently possible
> 
>   OR
> 
> I can put it on the left side of the tabbar in whatever form you want it to
> take.
> 
> I cannot do both because I do not have the necessary skills or knowledge of how
> all of this works to add a new toolbar to the customization scheme.  However,
> it is also my opinion that even if I did, such a patch would be rejected anyway
> for being too late for the feature freeze.
> 
> SO although UX may have spoken, I think we need a new answer.  If none is
> forthcoming then I guess I can't help here.

I vote door #2: left side of tabbar when there's no normal menu with no placement customization. Customization is nifty, but I care way less about that then just getting the button in. Just make things act vaguely similar to Windows XP and I'm happy.
Comment 85 Bill Gianopoulos [:WG9s] 2010-09-08 15:46:20 PDT
(In reply to comment #81)
> Not with any window manager :) but even the real firefox logo there wouldn't
> really scream "click me I am a menu button" to me.

OK, would it seem any better to you with a tooltip?

I am realy not trying to be agumentative here.  I just really don't know how to implement what is asked for in comment 69.  However I don't see anyone else stepping up trying to help here at all.
Comment 86 Bill Gianopoulos [:WG9s] 2010-09-08 16:17:41 PDT
(In reply to comment #73)
> (In reply to comment #71)
> > There are issues with this approach.  Most notably that it requires supporting
> > the placement of toolbar icons on the bookmarks toolbar which is not currently
> > supported and last I knew feature freeze was scheduled for Friday.
> 
> I don't understand what you mean by this. You can currently place toolbar icons
> on the bookmarks toolbar.

Oops.  I meant to say on  the tabsbar, which is currently NOT supported.
Comment 87 Stephen Horlander [:shorlander] 2010-09-08 16:21:23 PDT
The tabbar has been a normal toolbar for a while and accepts other toolbar items :)
Comment 88 Bill Gianopoulos [:WG9s] 2010-09-08 16:24:36 PDT
(In reply to comment #87)
> The tabbar has been a normal toolbar for a while and accepts other toolbar
> items :)

It does but only to be added to the right of the normally defined items.  There is absolutely no way to us the customize toolbars menu to add anything to the left of the tabs, which is, i presume what you really want here.
Comment 89 Stephen Horlander [:shorlander] 2010-09-08 16:26:21 PDT
(In reply to comment #88)
> (In reply to comment #87)
> > The tabbar has been a normal toolbar for a while and accepts other toolbar
> > items :)
> 
> It does but only to be added to the right of the normally defined items.  There
> is absolutely no way to us the customize toolbars menu to add anything to the
> left of the tabs, which is, i presume what you really want here.

Ah yes, that would be a problem.
Comment 90 Bill Gianopoulos [:WG9s] 2010-09-08 16:29:20 PDT
(In reply to comment #89)
> (In reply to comment #88)
> > (In reply to comment #87)
> > > The tabbar has been a normal toolbar for a while and accepts other toolbar
> > > items :)
> > 
> > It does but only to be added to the right of the normally defined items.  There
> > is absolutely no way to us the customize toolbars menu to add anything to the
> > left of the tabs, which is, i presume what you really want here.
> 
> Ah yes, that would be a problem.

But then I guess you are correct.  Maybe I really just need to look into this farther to see why things I try to add on the left end up on the right.  This might be more easily doable than I thought.
Comment 91 Bill Gianopoulos [:WG9s] 2010-09-08 16:38:39 PDT
(In reply to comment #88)
> (In reply to comment #87)
> > The tabbar has been a normal toolbar for a while and accepts other toolbar
> > items :)
> 
> It does but only to be added to the right of the normally defined items.  There
> is absolutely no way to us the customize toolbars menu to add anything to the
> left of the tabs, which is, i presume what you really want here.

Just to be clear here.  I think you idea of how this should really work is really right on.  My issue was just with trying to work more with the realities of what can be done in the Firefox timeframe.  My initial feeling was it might need to be a we get it close for 4.0, maybe mavbe an extensions to fix it better and can do what you suggested for 4.1, but I think I need ot take another look at this.  I think it should be doable within the Firefox 4 timeframe, living within the feature/string freeze constraints.  just I am just not sure I am smart enough to figure out how.  But I am going to give it another look.

I have some promising ideas, now that I think on it some more.
Comment 92 Dave Garrett 2010-09-08 17:05:05 PDT
(In reply to comment #88)
> (In reply to comment #87)
> > The tabbar has been a normal toolbar for a while and accepts other toolbar
> > items :)
> 
> It does but only to be added to the right of the normally defined items.  There
> is absolutely no way to us the customize toolbars menu to add anything to the
> left of the tabs, which is, i presume what you really want here.

I don't know where you're getting that idea. Just today, on Minefield under Linux, I moved the home button up to the left of the tab bar, specifically to the left of an app tab. It works fine.

Just for completeness-sake, I tried a new profile with latest Minefield and it lets me use the customize toolbar dialog to put any button I want on the left or right of the tab bar. Just note that by "tab bar" I mean the toolbar which contains the tabs and by "tabs" here I include the empty space where more new tabs are added (meaning all buttons on the right are aligned to the far right after the tab space). Simply put, you can customize this like anywhere else just fine.
Comment 93 Bill Gianopoulos [:WG9s] 2010-09-08 17:10:24 PDT
(In reply to comment #92)
> (In reply to comment #88)
> > (In reply to comment #87)
> > > The tabbar has been a normal toolbar for a while and accepts other toolbar
> > > items :)
> > 
> > It does but only to be added to the right of the normally defined items.  There
> > is absolutely no way to us the customize toolbars menu to add anything to the
> > left of the tabs, which is, i presume what you really want here.
> 
> I don't know where you're getting that idea. Just today, on Minefield under
> Linux, I moved the home button up to the left of the tab bar, specifically to
> the left of an app tab. It works fine.
> 
> Just for completeness-sake, I tried a new profile with latest Minefield and it
> lets me use the customize toolbar dialog to put any button I want on the left
> or right of the tab bar. Just note that by "tab bar" I mean the toolbar which
> contains the tabs and by "tabs" here I include the empty space where more new
> tabs are added (meaning all buttons on the right are aligned to the far right
> after the tab space). Simply put, you can customize this like anywhere else
> just fine.

Well that is even better news. if you can already do this as long as an app tab is present, then I can just look at how the app tabs work.  I cannot get this to work without app tabs already present but will look into that.  Sound like I am either doing something wrong.  App tabs being present makes a difference or perhaps it is extension related.  Anyway this all gives me hope of being able to make this work as specified without violating any of the feature freeze rules.
Comment 94 Dave Garrett 2010-09-08 17:15:23 PDT
(In reply to comment #93)
> Well that is even better news. if you can already do this as long as an app tab
> is present, then I can just look at how the app tabs work.

I did it the first time with an app tab just happening to be there. (mentioning it was an accidental red herring on my part) In the second part of comment 92 I said I just tested in a new profile, no app tabs involved.

It all works naturally. Maybe you're just not dragging it to just the right spot and it's finicky?
Comment 95 Bill Gianopoulos [:WG9s] 2010-09-08 17:23:29 PDT
(In reply to comment #94)
> (In reply to comment #93)
> > Well that is even better news. if you can already do this as long as an app tab
> > is present, then I can just look at how the app tabs work.
> 
> I did it the first time with an app tab just happening to be there. (mentioning
> it was an accidental red herring on my part) In the second part of comment 92 I
> said I just tested in a new profile, no app tabs involved.
> 
> It all works naturally. Maybe you're just not dragging it to just the right
> spot and it's finicky?

OIC, this is lame, but it does work.  With just tabs there you can not place something to the left of the leftmost tab by tying to place it to the left of it, but trying to place it on top of it kind of ends up placing it to the left.  OK.  I guess I can go with that if that is how it works.  I will try to come up with a patch as time permits.
Comment 96 Dão Gottwald [:dao] 2010-09-09 09:01:43 PDT
"Firefox" as the label makes a lot of sense when it replaces the window caption, less so when the window caption already says "Firefox" and the button is somewhere on the toolbars. "Menu" would probably make more sense there. We might want to honor the icon/text/icon+text mode state as well.
Comment 97 Bill Gianopoulos [:WG9s] 2010-09-09 15:34:36 PDT
(In reply to comment #96)
> "Firefox" as the label makes a lot of sense when it replaces the window
> caption, less so when the window caption already says "Firefox" and the button
> is somewhere on the toolbars. "Menu" would probably make more sense there. We
> might want to honor the icon/text/icon+text mode state as well.

That makes sense to me.  Should we do a patch to just define the "Menu" label in the appropriate properties file so that that part of this makes the localization freeze deadline?
Comment 98 Bill Gianopoulos [:WG9s] 2010-09-09 16:01:11 PDT
Created attachment 473794 [details] [diff] [review]
Just the l10n piece of this

This is just the part of this that would need to land before localization freeze if we want to go with Menu as the label for the toolbar button.
Comment 99 Dão Gottwald [:dao] 2010-09-10 06:34:08 PDT
Comment on attachment 473794 [details] [diff] [review]
Just the l10n piece of this

All other toolbar buttons have a tooltip, this should probably have one as well. That might actually be of use for Windows as well.
Comment 100 Bill Gianopoulos [:WG9s] 2010-09-10 09:17:54 PDT
(In reply to comment #99)
> Comment on attachment 473794 [details] [diff] [review]
> Just the l10n piece of this
> 
> All other toolbar buttons have a tooltip, this should probably have one as
> well. That might actually be of use for Windows as well.

OK.  What should the tooltip say?

"Display appShortname menu" ?
I take it your point is to decide on the tooltip also so we can get all the l10n strings in before freeze.  Sounds like a plan.
Comment 101 Bill Gianopoulos [:WG9s] 2010-09-10 09:20:42 PDT
(In reply to comment #100)
> (In reply to comment #99)
> > Comment on attachment 473794 [details] [diff] [review] [details]
> > Just the l10n piece of this
> > 
> > All other toolbar buttons have a tooltip, this should probably have one as
> > well. That might actually be of use for Windows as well.
> 
> OK.  What should the tooltip say?
> 
> "Display appShortname menu" ?
> I take it your point is to decide on the tooltip also so we can get all the
> l10n strings in before freeze.  Sounds like a plan.

I know sooner hit submit, than I realized it is not a good idea to try to make this a multi-segment thing with appShortname in the middle as you then run into difficulty with languages that have a completely different syntax as to where in a phrase different parts of speech go.
Comment 102 Dão Gottwald [:dao] 2010-09-10 11:59:53 PDT
<!ENTITY appMenuButton.tooltip "Open &brandShortName; menu"> should work.
Comment 103 :Ehsan Akhgari (busy, don't ask for review please) 2010-09-10 13:17:45 PDT
Comment on attachment 473794 [details] [diff] [review]
Just the l10n piece of this

I've included this patch in my patch in bug 593126.
Comment 104 Bill Gianopoulos [:WG9s] 2010-09-10 14:05:45 PDT
(In reply to comment #103)
> Comment on attachment 473794 [details] [diff] [review]
> Just the l10n piece of this
> 
> I've included this patch in my patch in bug 593126.

OK, but you patch does not include the tooltip I was going to add to the patch here.
Comment 105 Alex Limi (:limi) — Firefox UX Team 2010-09-10 15:04:19 PDT
(In reply to comment #96)
> "Firefox" as the label makes a lot of sense when it replaces the window
> caption, less so when the window caption already says "Firefox" and the button
> is somewhere on the toolbars. "Menu" would probably make more sense there. We
> might want to honor the icon/text/icon+text mode state as well.

That doesn't make sense to me. It's called the Firefox menu for a reason. :)

This is straying into Opera territory, where when they got horrible results from just having their menu be their "O" icon, they just added the word "Menu" and put the entire existing menu tree under it.

If you think there's any chance of confusing a menu with a window title, we should do the opposite — remove "Firefox" from the window title, like we do on other platforms. (I assume the application switcher icon can still show the application name on Linux?)
Comment 106 Alex Faaborg [:faaborg] (Firefox UX) 2010-09-10 15:09:00 PDT
I agree with everything Limi said.  I would also add that this is called "the firefox menu" if it doesn't have a name you can't tell someone to go to it, or really explain to anyone that it exists. (or less commonly, file a bug on it).
Comment 107 Dão Gottwald [:dao] 2010-09-10 15:10:54 PDT
(In reply to comment #105)
> (In reply to comment #96)
> > "Firefox" as the label makes a lot of sense when it replaces the window
> > caption, less so when the window caption already says "Firefox" and the button
> > is somewhere on the toolbars. "Menu" would probably make more sense there. We
> > might want to honor the icon/text/icon+text mode state as well.
> 
> That doesn't make sense to me. It's called the Firefox menu for a reason. :)
> 
> This is straying into Opera territory, where when they got horrible results
> from just having their menu be their "O" icon, they just added the word "Menu"
> and put the entire existing menu tree under it.

I'm not sure what all this really means. Are you saying that if we labeled it "Menu", we'd have to follow Opera with regards to the menu structure?

> If you think there's any chance of confusing a menu with a window title, we
> should do the opposite — remove "Firefox" from the window title, like we do on
> other platforms. (I assume the application switcher icon can still show the
> application name on Linux?)

"other platforms" would be Mac -- we actually have "Firefox" in the window title on Windows, like every other browser except Safari.
Comment 108 Dão Gottwald [:dao] 2010-09-10 15:13:26 PDT
(In reply to comment #106)
> I agree with everything Limi said.  I would also add that this is called "the
> firefox menu" if it doesn't have a name you can't tell someone to go to it, or
> really explain to anyone that it exists. (or less commonly, file a bug on it).

"The Firefox menu" seems to apply just fine here:

,-------------- Firefox ---------------.
| [Menu v]                             |
|                                      |
...
Comment 109 :Ehsan Akhgari (busy, don't ask for review please) 2010-09-10 15:29:18 PDT
(In reply to comment #104)
> (In reply to comment #103)
> > Comment on attachment 473794 [details] [diff] [review]
> > Just the l10n piece of this
> > 
> > I've included this patch in my patch in bug 593126.
> 
> OK, but you patch does not include the tooltip I was going to add to the patch
> here.

In that case please include the tooltip on a patch on this bug.
Comment 110 Alex Limi (:limi) — Firefox UX Team 2010-09-10 15:31:33 PDT
(In reply to comment #107)
> I'm not sure what all this really means. Are you saying that if we labeled it
> "Menu", we'd have to follow Opera with regards to the menu structure?

No. It's like labeling a window with "Window". It doesn't make sense. :)

> "other platforms" would be Mac -- we actually have "Firefox" in the window
> title on Windows, like every other browser except Safari.

Only on XP at this point. Vista/7 doesn't have it. In any case, I'm not arguing for doing this, I'm just saying that if you're concerned about having "Firefox" in two locations, we should prefer getting rid of it from the title bar over naming the menu "Menu".
Comment 111 Dão Gottwald [:dao] 2010-09-10 15:39:03 PDT
(In reply to comment #110)
> (In reply to comment #107)
> > I'm not sure what all this really means. Are you saying that if we labeled it
> > "Menu", we'd have to follow Opera with regards to the menu structure?
> 
> No. It's like labeling a window with "Window". It doesn't make sense. :)

Well, things need to have distinct labels. In an environment with multiple windows, "window" obviously doesn't cut it. There's no such ambiguity in a window such as the one in comment 108.

> > "other platforms" would be Mac -- we actually have "Firefox" in the window
> > title on Windows, like every other browser except Safari.
> 
> Only on XP at this point. Vista/7 doesn't have it. In any case, I'm not arguing
> for doing this, I'm just saying that if you're concerned about having "Firefox"
> in two locations, we should prefer getting rid of it from the title bar over
> naming the menu "Menu".

Vista/7 do have it, it just not normally visible when we render the Firefox button instead of the stock title bar.
I would also add that removing "Firefox" from the window title seems to be beyond this bug's scope.
Comment 112 Alex Limi (:limi) — Firefox UX Team 2010-09-10 15:44:34 PDT
(In reply to comment #111)
> Vista/7 do have it, it just not normally visible when we render the Firefox
> button instead of the stock title bar.
> I would also add that removing "Firefox" from the window title seems to be
> beyond this bug's scope.

So is renaming the menu, to be honest. Let's get the basic implementation working the same as on the other platforms as far as capability and general layout goes (not placement, obviously).
Comment 113 Dão Gottwald [:dao] 2010-09-11 00:52:15 PDT
(In reply to comment #112)
> So is renaming the menu, to be honest. Let's get the basic implementation
> working the same as on the other platforms as far as capability and general
> layout goes (not placement, obviously).

I didn't say anything about capability or the menu layout. Beyond that please don't assert consistency where it just doesn't exist. "Other platforms" means Windows in this case and we're replacing the title bar with the Firefox button there. It's a different setup.
Comment 114 Alex Faaborg [:faaborg] (Firefox UX) 2010-09-11 20:49:38 PDT
One of our goals with providing users with this option is to allow them to have a firefox interface that has cross platform consistency.  This is also important in terms of our documentation.
Comment 115 Dão Gottwald [:dao] 2010-09-12 01:37:00 PDT
Ease of documentation probably shouldn't be a driving factor here, we should just do what makes most sense. "Firefox" being duplicated and scattered over the UI surely doesn't make a lot of sense.

Cross-platform consistency is moot right now. The interface will in fact be different, given the title bar and keyboard access constraints. Neither will the Linux/OS X/Windows transition be seamless nor will we able to document it as such. The optional button on the toolbar for Linux is essentially a stopgap rather than an achieved goal. Long-term I'd even expect us to drop it if/when Gnome gets a global and mandatory menu bar like OS X.
Comment 116 Dave Garrett 2010-09-12 06:32:03 PDT
(In reply to comment #114)
> One of our goals with providing users with this option is to allow them to have
> a firefox interface that has cross platform consistency.  This is also
> important in terms of our documentation.

Yes and yes.

(In reply to comment #115)
> Ease of documentation probably shouldn't be a driving factor here

The word "documentation" here could be swapped out for some form of "users figuring out how to use the thing". That includes docs, help, help relying on docs, and users just winging it. If Firefox varies massively in base (w/o addons) capability depending on where you go then it no longer becomes a stable and consistent single product. I'd go as far as to say that you shouldn't be using the same name if you don't have the same main base capabilities (regardless of the defaults and minor platform specific differences).

(In reply to comment #115)
> Long-term I'd even expect us to drop it if/when
> Gnome gets a global and mandatory menu bar like OS X.

And us KDE users are what, screwed? (not Qt; GTK on KDE) Not to mention the other DEs out there.

Gnome !== Linux

> The interface will in fact be different

Different, but with similar capabilities (one would hope), mostly just different defaults and hotkeys. Same stuff with a different style. We can debate about the style but I want to keep the same stuff, and to be explicit, the Firefox button is stuff not style.
Comment 117 Dave Garrett 2010-09-12 06:34:13 PDT
> the Firefox button is stuff not style.

Its usage, including being on by default, is style.
Comment 118 Alex Faaborg [:faaborg] (Firefox UX) 2010-09-13 10:25:23 PDT
The Firefox button originated on Windows since we don't have a menu bar there.  However, I expect it to show up on other platforms where the user may not be familiar with the surrounding OS, or they might not even have access to the surrounding OS.  For instance, I could imagine that Firefox running in full screen on larger format android tablets or netbooks might use a Firefox button. (the same could potentially be true for some linux distributions on netbooks, especially if they have a custom launcher interface).  So in this regard, cross platform consistency is actually an objective.
Comment 119 Bill Gianopoulos [:WG9s] 2010-09-18 13:10:26 PDT
Created attachment 476569 [details] [diff] [review]
l10n patch required for tooltip on toolbar button (checked in)

My real job schedule seems to have permitted me once again to find time to work on this again.  I will probably just leave it assigned to me form now on as when work interrupts, no one else seems to step up to working this anyway.

I almost have the movable toolbar item working and will probably be posting a patch for that soon.  The menubar hiding is probably simpler so that patch should follow soon after.

However, I just realized I never asked to have this reviewed and checked-in and this actually need to land before the localization freeze which is imminent.
Comment 120 antistress 2010-09-20 15:23:50 PDT
Stephen Horlander said (c#4) "The ability to natively draw widgets in the titelbar should come with gtk3"
Aren't you talking about client-side windows which has been implemented in GTK+ 2.18 ?
http://blogs.gnome.org/gtk/2009/07/10/gtk-2173-unstable-release/
Comment 121 Michael Monreal [:monreal] 2010-09-21 00:27:56 PDT
(In reply to comment #120)
> Aren't you talking about client-side windows which has been implemented in GTK+
> 2.18 ?

No, we were talking about client side window *decorations* which are even not yet ready for GTK 3.0... :(
Comment 122 David Tenser [:djst] 2010-09-21 03:49:44 PDT
(In reply to comment #118)
> The Firefox button originated on Windows since we don't have a menu bar there. 
> However, I expect it to show up on other platforms where the user may not be
> familiar with the surrounding OS, or they might not even have access to the
> surrounding OS. [...] So in this regard, cross
> platform consistency is actually an objective.

I would also add that the coolness factor of having a Firefox button instead of the traditional menu should not be underestimated. One of the most common complaints in Input on Linux has been the lack of a Firefox button, which makes me believe that having the button enabled by default would be preferred by most early adopters.
Comment 123 Nicolas Mandil (:NicolasWeb) 2010-09-21 04:07:53 PDT
(In reply to comment #122)
> (In reply to comment #118)
> > The Firefox button originated on Windows since we don't have a menu bar there. 
> > However, I expect it to show up on other platforms where the user may not be
> > familiar with the surrounding OS, or they might not even have access to the
> > surrounding OS. [...] So in this regard, cross
> > platform consistency is actually an objective.
> 
> I would also add that the coolness factor of having a Firefox button instead of
> the traditional menu should not be underestimated. One of the most common
> complaints in Input on Linux has been the lack of a Firefox button, which makes
> me believe that having the button enabled by default would be preferred by most
> early adopters.

New bug for that : Bug 598293
Comment 124 Bill Gianopoulos [:WG9s] 2010-09-22 14:42:57 PDT
I had been holding off on posting the patches for this because I don't have check-in privileges and was trying to make sure that the wrong patch did not get checked-in with my checkin-needed request.  As it now seems there is a less than zero chance of getting the l10n part of this checked-in before the beta7 localization string freeze, I will be posting the patches i have to fix this later tonight.
Comment 125 Bill Gianopoulos [:WG9s] 2010-09-22 17:32:36 PDT
Created attachment 477786 [details] [diff] [review]
Implement applications menu as a toolbar button

This patch does the following:

1.  It has no effect whatsoever on Windows builds.

2.  It implements the Application Menu button as a customizable toolbar button that can be placed on any customizable toolbar in any position chosen by the user.

3.  In order to permit the menu to be used either in the titlebar, or as a customizable toolbar item, I moved the appmenu-popup definition to an include file much as the menubar currently is.  This allows the same menu to be used in both places while changes only need to be made in one place in the source.

4.  Under Linux, by default, the Application Menu button is placed at the left of the tabbar.

5.  Under Mac/OSX the button, by default is not displayed.

6.  The toolbar button icon is currently just a placeholder.  I fully expect that the Linux theme group will change this to a more appropriate icon after this is checked in.  I am not an artistic or theme person, I am just trying to get this working as desired.

7.  For menu icons, on the new application button menu, I use the same icons as those used for the equivalent choices on the traditional menubar menu.

8.  For now, since I have no access to a MAC and no way to test what this looks like, since I had no facilities to do any pinstripe theme changes, I have temporarily placed an #ifndef XP_MACOSX in the code to disable the toolbar item under MAC/OSX.  This is noted with an appropriate XXX comment.
Comment 126 Alex Limi (:limi) — Firefox UX Team 2010-09-22 17:46:45 PDT
(In reply to comment #125)
> Created attachment 477786 [details] [diff] [review]
> Implement applications menu as a toolbar button

Cool, could you attach a screenshot for those of us that don't have a Linux box to test this? :)
Comment 127 Bill Gianopoulos [:WG9s] 2010-09-22 17:57:11 PDT
Created attachment 477793 [details] [diff] [review]
Implement menubar autohide under Linux

With this I ran into an issue.

Perhaps I am stupid, but I was unable to find any way to not display the menubar that did not end up leaving the hotkeys ALT-F, ALT-E, ALT-V, ALT-H, ALT-B, ALT-T and ALT-H active and still displaying the menu popups without the menubar re-showing.  This seemed worse than just re-activating the autohide for Linux and having those keys both show the pop-up and unhide the menubar.

When this code was previously disabled it was because there was no reasonable way to un-hide the menubar.  Now that we are activating the Application menu button as a toollbar item under Linux, the reasonable way to un-hide the menubar is to use Options -> Menu Bar from that button menu.
Comment 128 Bill Gianopoulos [:WG9s] 2010-09-22 18:05:43 PDT
Created attachment 477794 [details]
screenshot showing button on the tabbar

Screenshot showing the menu button on the tabbar.  I tried to show this with the menu open but stupid Linux won;t allow me to do a screenshot with a popup menu displayed.  Go figure.
Comment 129 Bill Gianopoulos [:WG9s] 2010-09-22 18:09:45 PDT
Created attachment 477797 [details]
screenshot showing toolbar in customize toolbar window
Comment 130 Bill Gianopoulos [:WG9s] 2010-09-22 18:11:03 PDT
There are also 32-bit and 64-bit builds available including all 3 patches on this bug at http://www.wg9s.com/mozilla/firefox/
Comment 131 Bill Gianopoulos [:WG9s] 2010-09-22 18:27:02 PDT
Created attachment 477804 [details]
screenshot with menu open

I used a camera
Comment 132 Dave Garrett 2010-09-22 18:43:36 PDT
(In reply to comment #128)
> stupid Linux won;t allow me to do a screenshot with a popup
> menu displayed.  Go figure.

Just use whatever screenshot application comes with your desktop environment. On KDE 4.4 here I get to KSnapshot by just pressing the print screen key. Just set it to a couple second delay, click "new snapshot", then click the menu within those couple seconds and wait. Poof, screenshot with menu open. (though, no cursor) I'm sure GNOME and others have something similar.

(In reply to comment #131)
> I used a camera

Hacky, but effective. :p
Comment 133 Bill Gianopoulos [:WG9s] 2010-09-22 18:53:42 PDT
Created attachment 477810 [details]
screenshot with menu open
Comment 134 Bill Gianopoulos [:WG9s] 2010-09-22 18:56:38 PDT
Comment on attachment 477810 [details]
screenshot with menu open

from terminal window:

gnome-screenshot --delay=5
Comment 135 Bill Gianopoulos [:WG9s] 2010-09-22 19:01:59 PDT
Created attachment 477813 [details]
screenshot showing button tooltip
Comment 136 Alex Faaborg [:faaborg] (Firefox UX) 2010-09-22 19:16:14 PDT
Comment on attachment 477794 [details]
screenshot showing button on the tabbar

If we want to go ahead and get this landed, that's totally fine.  However, eventually we need to address the following things with the UI:

-The button should be called "Firefox" in the customization pallet, so that it can be consistently referred to as the Firefox button, which produces the Firefox menu.
-The button should either be a Firefox icon, or the text "Firefox" depending on placement, it shouldn't be a down arrow
-The menu should not contain keyboard shortcuts on the top level, these are already exposed on each of the sub-menus
Comment 137 Michael Monreal [:monreal] 2010-09-23 00:59:18 PDT
(In reply to comment #136)
> -The button should either be a Firefox icon, or the text "Firefox" depending on
> placement, it shouldn't be a down arrow

Won't this look bad in combination with the simple icon style which I think is still meant to make it into 4.0? Maybe we can have simplified glyph for this as well (which would probably work better at the small size, too)

Anyway, this should land ASAP, the look can still be polished later.
Comment 138 Dão Gottwald [:dao] 2010-09-23 01:48:21 PDT
(In reply to comment #137)
> Anyway, this should land ASAP, the look can still be polished later.

It will land when it's ready.
Comment 139 Dão Gottwald [:dao] 2010-09-23 01:49:06 PDT
Comment on attachment 476569 [details] [diff] [review]
l10n patch required for tooltip on toolbar button (checked in)

http://hg.mozilla.org/mozilla-central/rev/9631d2f6c6a0
Comment 140 Dão Gottwald [:dao] 2010-09-23 01:51:40 PDT
(In reply to comment #136)
> -The button should either be a Firefox icon, or the text "Firefox" depending on
> placement, it shouldn't be a down arrow

The arrow doesn't make sense, but we certainly don't want a Firefox icon here either. A Firefox icon is right above this in the title bar (unless your Gtk theme removes it).
Comment 141 Dão Gottwald [:dao] 2010-09-23 02:00:11 PDT
Comment on attachment 477786 [details] [diff] [review]
Implement applications menu as a toolbar button

The way MENUBAR_CAN_AUTOHIDE is handled here doesn't seem to make sense. Seems like it should be defined on Linux.

>+++ b/browser/base/content/browser-appmenu.inc

>+# The Initial Developer of the Original Code is
>+# Bill Gianopoulos <bill@wg9s.com>.

You're really just moving it. ;-)

>+#ifdef MOZ_WIDGET_GTK2
>+             defaultset="appmenu-button,tabbrowser-tabs,new-tab-button,alltabs-button,tabview-button,tabs-closebutton"
>+#else
>              defaultset="tabbrowser-tabs,new-tab-button,alltabs-button,tabview-button,tabs-closebutton"
>+#endif
>              collapsed="true">
> 
>+#ifndef MENUBAR_CAN_AUTOHIDE
>+#ifndef XP_MACOSX
>+# XXX: bug 585370 -  temporarily disable under MAC/OSX until pinstripe theme is
>+#                    updated to theme the appmenu-button

Make this ifdef MOZ_WIDGET_GTK2, remove the XXX comment. We can't hide the menu bar on OS X.

>+      <toolbarbutton id="appmenu-button"
>+                     class="toolbarbutton-1 chromeclass-toolbar-additional"
>+                     type="menu"
>+                     label="&appMenuButton.label;"
>+                     tooltiptext="&appMenuButton.tooltip;"
>+                     removable="true">
>+#include browser-appmenu.inc
>+      </toolbarbutton>
>+#endif
>+#endif

We need to make sure the button shows up as the user hides the menu bar (but not otherwise, which your patch doesn't seem to handle?). Making it removable seems like a footgun we better shouldn't provide.
Comment 142 Dão Gottwald [:dao] 2010-09-23 02:02:14 PDT
Comment on attachment 477793 [details] [diff] [review]
Implement menubar autohide under Linux

>diff --git a/browser/base/content/browser.xul b/browser/base/content/browser.xul
>--- a/browser/base/content/browser.xul
>+++ b/browser/base/content/browser.xul
>@@ -810,17 +471,17 @@
>            tabsontop="true"
> #endif
>            persist="tabsontop">
>     <!-- Menu -->
>     <toolbar type="menubar" id="toolbar-menubar" class="chromeclass-menubar" customizable="true"
>              defaultset="menubar-items"
>              mode="icons" iconsize="small" defaulticonsize="small"
>              lockiconsize="true"
>-#ifdef MENUBAR_CAN_AUTOHIDE
>+#ifndef XP_MACOSX
>              toolbarname="&menubarCmd.label;"
>              accesskey="&menubarCmd.accesskey;"
> #endif
>              context="toolbar-context-menu">

Again, update MENUBAR_CAN_AUTOHIDE to reflect the new reality.
Comment 143 Dão Gottwald [:dao] 2010-09-23 02:08:14 PDT
Maybe we should use an icon like this one:

http://mxr.mozilla.org/mozilla-central/source/browser/themes/gnomestripe/browser/places/bookmarksMenu.png

In fact, for now you could start referencing this very icon. We don't need a large version as the tab bar's icon size is always small.

It also seems like you shouldn't hide the dropmarker.
Comment 144 Bill Gianopoulos [:WG9s] 2010-09-23 03:15:40 PDT
(In reply to comment #136)
> Comment on attachment 477794 [details]
> screenshot showing button on the tabbar
> 
> If we want to go ahead and get this landed, that's totally fine.  However,
> eventually we need to address the following things with the UI:
> 
> -The button should be called "Firefox" in the customization pallet, so that it
> can be consistently referred to as the Firefox button, which produces the
> Firefox menu.
> -The button should either be a Firefox icon, or the text "Firefox" depending on

Hmm in my earlier mock-ups I had posted in this patch I was using the firefox icon and I thought that idea had been shot-down.

> placement, it shouldn't be a down arrow

As I said, this was just a placeholder and I was not really trying to come-up with the final icon in this bug.  If we want to do that, however, I am ok doing that, if that is what is needed to get this landed.

> -The menu should not contain keyboard shortcuts on the top level, these are
> already exposed on each of the sub-menus

Hmm.  Oddly, I did not notice it was doing that.  I will look at how Windows is hiding those and copy the code.  It did not occur to me to check since I am using the same popup definition that windows does.
Comment 145 Bill Gianopoulos [:WG9s] 2010-09-23 03:37:53 PDT
(In reply to comment #141)
> Comment on attachment 477786 [details] [diff] [review]
> Implement applications menu as a toolbar button
> 
> The way MENUBAR_CAN_AUTOHIDE is handled here doesn't seem to make sense. Seems
> like it should be defined on Linux.

I had noticed that as well.  The problem is that after all of this lands, assuming it does.  This ifdef is still required but is really unrelated to if the menubar can autohide or not.  I think the real issue is that part of what it controlled originally had to do with autohide, and part of what it did had to do with having showing or hiding the menubar controlling the visibility of the Firefox button.  Which now only works under windows.  I had planned on filing a followup bug on renaming this DEFINE, but could do it as part of this bug also.

It should probably be called MENU_BUTTON_BAR_TOGGLE or something to that effect.

> 
> >+++ b/browser/base/content/browser-appmenu.inc
> 
> >+# The Initial Developer of the Original Code is
> >+# Bill Gianopoulos <bill@wg9s.com>.
> 
> You're really just moving it. ;-)

That is true.  Who did the original app button code?  should probably put them here.

> 
> >+#ifdef MOZ_WIDGET_GTK2
> >+             defaultset="appmenu-button,tabbrowser-tabs,new-tab-button,alltabs-button,tabview-button,tabs-closebutton"
> >+#else
> >              defaultset="tabbrowser-tabs,new-tab-button,alltabs-button,tabview-button,tabs-closebutton"
> >+#endif
> >              collapsed="true">
> > 
> >+#ifndef MENUBAR_CAN_AUTOHIDE
> >+#ifndef XP_MACOSX
> >+# XXX: bug 585370 -  temporarily disable under MAC/OSX until pinstripe theme is
> >+#                    updated to theme the appmenu-button
> 
> Make this ifdef MOZ_WIDGET_GTK2, remove the XXX comment. We can't hide the menu
> bar on OS X.

The current ifndef would make more sense if i renamed the MENUBAR_CAN_AUTOHIDE DEFINE to describe its current function.  THe reason I had this doing OS X as well was that about a zillion comments back, where the goal here was defined, the idea promoted was to make this a customizable toolbar button on al OS's except Windows.  I could just use MOZ_WIDGET_GTK2.  I was just trying to help out the OS X guys buy having this in place for them to enable once they did the theme piece.  I can make this Linux only as this is a Linux only bug.
> 
> >+      <toolbarbutton id="appmenu-button"
> >+                     class="toolbarbutton-1 chromeclass-toolbar-additional"
> >+                     type="menu"
> >+                     label="&appMenuButton.label;"
> >+                     tooltiptext="&appMenuButton.tooltip;"
> >+                     removable="true">
> >+#include browser-appmenu.inc
> >+      </toolbarbutton>
> >+#endif
> >+#endif
> 
> We need to make sure the button shows up as the user hides the menu bar (but
> not otherwise, which your patch doesn't seem to handle?). Making it removable
> seems like a footgun we better shouldn't provide.

If I do make it work that way, then all my comments about MENUBAR_CAN_AUTOHIDE above do not apply.  I just re-read the desired behavior specified in comment 69 and it does say the toolbar item should show if the menubar is hidden.  I guess that means the visibility of this button should depend on the menubar just as the Windows button does.  SO I will try to get that working.
Comment 146 Bill Gianopoulos [:WG9s] 2010-09-23 03:47:12 PDT
(In reply to comment #143)
> Maybe we should use an icon like this one:
> 
> http://mxr.mozilla.org/mozilla-central/source/browser/themes/gnomestripe/browser/places/bookmarksMenu.png
> 
> In fact, for now you could start referencing this very icon. We don't need a
> large version as the tab bar's icon size is always small.
> 
> It also seems like you shouldn't hide the dropmarker.

I thought the placement on the tabbar was supposed to be the default location, but the user was supposed to be able to place it on whatever toolbar they preferred.  If that is the case, then both sizes are required.  If we just want to have this non-customizable and always on tabbar left and visible when the menubar is hidden, I can do that as well.

The problem I had with the dropmarker was also related to making this customizable.  Using the style for a customizable button seemed to lock the width so that the dropmarker displayed below the icon.  That is why I removed it, and also why I changed the placemarker icon I was using to the down arrow on the idea that it at least looked  a bit like a dropmarker.

I guess I need to try to figure out how to get the dropmarker on the right.
Comment 147 Dão Gottwald [:dao] 2010-09-23 03:55:24 PDT
> > >+# The Initial Developer of the Original Code is
> > >+# Bill Gianopoulos <bill@wg9s.com>.
> > 
> > You're really just moving it. ;-)
> 
> That is true.  Who did the original app button code?  should probably put them
> here.

"the Mozilla Organization" did...
Please add me (bug 570795) and Joshua M (bug 585370) as contributors.

> If I do make it work that way, then all my comments about MENUBAR_CAN_AUTOHIDE
> above do not apply.  I just re-read the desired behavior specified in comment
> 69 and it does say the toolbar item should show if the menubar is hidden.  I
> guess that means the visibility of this button should depend on the menubar
> just as the Windows button does.

Exactly.

(In reply to comment #146)
> I thought the placement on the tabbar was supposed to be the default location,
> but the user was supposed to be able to place it on whatever toolbar they
> preferred.  If that is the case, then both sizes are required.  If we just want
> to have this non-customizable and always on tabbar left and visible when the
> menubar is hidden, I can do that as well.

See the last part of comment 141. Also, the user can't move it on Windows, so locking it here seems fair.
Comment 148 Dão Gottwald [:dao] 2010-09-23 03:57:45 PDT
> "the Mozilla Organization" did...

Err, it's the Mozilla Foundation.
Comment 149 antistress 2010-09-23 04:05:39 PDT
(In reply to comment #146)
 
> I guess I need to try to figure out how to get the dropmarker on the right.

I don't know if it can help but Nautilus has such dropmarker in its sidepanel
see here "Orte v" http://www.cedynamix.fr/wp-content/uploads/2008/07/nautilus-tabs.png
Comment 150 Michael Monreal [:monreal] 2010-09-23 04:15:02 PDT
(In reply to comment #147)
> See the last part of comment 141. Also, the user can't move it on Windows, so
> locking it here seems fair.

So much moving back and forth here... on windows, the button does not take away
tab space, so not being movable is not a big deal. Being part of the toolbar on
linux, it would be very bad to not be able to customize it. And customizing
includes removing in this case.

(In reply to comment #146)
> I guess I need to try to figure out how to get the dropmarker on the right.

You can probably copy it from the bookmarks menu button?
Comment 151 Bill Gianopoulos [:WG9s] 2010-09-23 06:44:00 PDT
(In reply to comment #148)
> > "the Mozilla Organization" did...
> 
> Err, it's the Mozilla Foundation.

(In reply to comment #148)
> > "the Mozilla Organization" did...
> 
> Err, it's the Mozilla Foundation.

Oops typo.  I will fix that.

I should be able to come up with new patches addressing all the review comments over the weekend.
Comment 152 Bill Gianopoulos [:WG9s] 2010-09-25 14:04:04 PDT
(In reply to comment #136)
> Comment on attachment 477794 [details]
> screenshot showing button on the tabbar

> -The menu should not contain keyboard shortcuts on the top level, these are
> already exposed on each of the sub-menus

I ran into an unexpected issue in trying to fix this.  I was originally under the impression that the issue here was that somehow there was something in winstripe that hid these, or that whatever else hid these did not work on a toolbar button.

The current code that controls this under Windows appears to be that checked in for bug 589139.

Anyway, in trying to prove this was not a it does not work under Linux issue, buy just doing a vanilla build including may previously posted patch from attachment 465030 [details] [diff] [review], which basically makes the linux build work as closely as possible to the windows version resulted in the menu still persisting in displaying the keyboard shortcuts.  So, it would seem that the fix in bug 589139 is for some reason windows only.

Anyway, I have CC'd margaret to see if we can figure out what the issue here is.
Comment 153 Bill Gianopoulos [:WG9s] 2010-09-25 14:44:37 PDT
Created attachment 478566 [details]
screenshot using bookmarkmenu.png icon
Comment 154 Bill Gianopoulos [:WG9s] 2010-09-25 14:47:19 PDT
(In reply to comment #143)
> Maybe we should use an icon like this one:
> 
> http://mxr.mozilla.org/mozilla-central/source/browser/themes/gnomestripe/browser/places/bookmarksMenu.png
> 
> In fact, for now you could start referencing this very icon. We don't need a
> large version as the tab bar's icon size is always small.

As you can see form attachment 478566 [details], it would seem that we need an icon with a higher saturation level than that one for something we are trying to showcase as new feature.  It looks way too washed out and almost looks disabled.
Comment 155 Dave Garrett 2010-09-25 14:55:39 PDT
Button styling of some kind as apposed to just the icon might help.

As you've said above, all the icons should be getting updated by theme experts so they can come up with something better later. It's decent enough looking for now, though, I think.
Comment 156 Bill Gianopoulos [:WG9s] 2010-09-25 14:57:53 PDT
So from Stephan Horlander comment #69 i have:

If the menubar is hidden manually the Firefox button appears next to the tabs
by default but can be moved anywhere.

However, Dão Gottwald seems to be favoring the the it is at at the left to the tabs bar solution, which is clearly easier to implement, and might even be all we can implement in time for Firefox4.

My problem with that is that if you don't have tabs on top selected, does it not make sense to be able to place this on the navigation bar?

In any event, I really do not have time to develop 2 solutions here.  I really need the UX team to come up with one clear direction that I will try to implement here.
Comment 157 Bill Gianopoulos [:WG9s] 2010-09-25 14:59:44 PDT
(In reply to comment #155)
> Button styling of some kind as apposed to just the icon might help.
> 
> As you've said above, all the icons should be getting updated by theme experts
> so they can come up with something better later. It's decent enough looking for
> now, though, I think.

It looks awful enough so that I refuse to use it for my personal builds.
Comment 158 Bill Gianopoulos [:WG9s] 2010-09-25 15:12:28 PDT
My point here is not to criticize the original suggestion of using this icon.  It looked fine to me until I tried to actually use it in context.

It is just that it really looks so bad in the context for which it is intended, that I don't want my name associated with any patch to use it in that context.
Comment 159 Dave Garrett 2010-09-25 15:16:11 PDT
(In reply to comment #156)
> However, Dão Gottwald seems to be favoring the the it is at at the left to the
> tabs bar solution, which is clearly easier to implement, and might even be all
> we can implement in time for Firefox4.
> 
> My problem with that is that if you don't have tabs on top selected, does it
> not make sense to be able to place this on the navigation bar?

Opting to select both old style tabs on bottom and also opting to add the new button doesn't seem like it would be a common combination. (Linux will have tabs on top as default whenever bug 593823 lands) I don't even think there'd be a problem with it in the tabs bar on the bottom if it's a small button. If it really is, you could just have it require tabs on top.

(In reply to comment #158)
> My point here is not to criticize the original suggestion of using this icon. 
> It looked fine to me until I tried to actually use it in context.
> 
> It is just that it really looks so bad in the context for which it is intended,
> that I don't want my name associated with any patch to use it in that context.

If you dislike the icon that much, just use the same orange Firefox button style from Windows until it can be updated to a good looking little icon in a followup bug.
Comment 160 Bill Gianopoulos [:WG9s] 2010-09-25 15:18:40 PDT
Created attachment 478581 [details] [diff] [review]
Current WIP patch to implement Firefox menu as a toolbar item
Comment 161 Dave Garrett 2010-09-25 15:21:16 PDT
Comment on attachment 476569 [details] [diff] [review]
l10n patch required for tooltip on toolbar button (checked in)

This attachment isn't obsolete if it wasn't backed out.
Comment 162 Bill Gianopoulos [:WG9s] 2010-09-25 15:29:22 PDT
(In reply to comment #147)
> Please add me (bug 570795) and Joshua M (bug 585370) as contributors.
                                 ^^^^^^^^^^^^^^^^^^^^^

I have been unable to ascertain a full name or email address for Joshua M in order to properly cite him as a contributor.  Also the bug number referenced here appears to be incorrect as it it is this bug.
Comment 163 Bill Gianopoulos [:WG9s] 2010-09-25 15:30:41 PDT
(In reply to comment #161)
> Comment on attachment 476569 [details] [diff] [review]
> l10n patch required for tooltip on toolbar button (checked in)
> 
> This attachment isn't obsolete if it wasn't backed out.

Guess I got carried away obsoleting old patches while posting the current WIP patch.
Comment 164 Bill Gianopoulos [:WG9s] 2010-09-25 15:36:09 PDT
Created attachment 478588 [details]
Screenshot showing icon used in WIP patch
Comment 165 Bill Gianopoulos [:WG9s] 2010-09-25 15:39:55 PDT
(In reply to comment #164)
> Created attachment 478588 [details]
> Screenshot showing icon used in WIP patch

OK.  There are issues with this patch as well.

1.  It is a stock gnome image.  If we decide to use this or an image based by this we need to make sure the licensing of the stock gnome images permits that.

2.  I intended to remove the arrow and try to make this look more like a restaurant menu.

3.  Although I originally intended this to look like a restaurant menu, I now realize it also looks like a book, and perhaps it might be confused with an icon for some bookmarks function.
Comment 166 Bill Gianopoulos [:WG9s] 2010-09-25 15:57:09 PDT
(In reply to comment #159)
> (In reply to comment #156)
> > However, Dão Gottwald seems to be favoring the the it is at at the left to the
> > tabs bar solution, which is clearly easier to implement, and might even be all
> > we can implement in time for Firefox4.
> > 
> > My problem with that is that if you don't have tabs on top selected, does it
> > not make sense to be able to place this on the navigation bar?
> 
> Opting to select both old style tabs on bottom and also opting to add the new
> button doesn't seem like it would be a common combination. (Linux will have
> tabs on top as default whenever bug 593823 lands) I don't even think there'd be
> a problem with it in the tabs bar on the bottom if it's a small button. If it
> really is, you could just have it require tabs on top.
> 
> (In reply to comment #158)
> > My point here is not to criticize the original suggestion of using this icon. 
> > It looked fine to me until I tried to actually use it in context.
> > 
> > It is just that it really looks so bad in the context for which it is intended,
> > that I don't want my name associated with any patch to use it in that context.
> 
> If you dislike the icon that much, just use the same orange Firefox button
> style from Windows until it can be updated to a good looking little icon in a
> followup bug.

I was just trying to get all the things I wanted to post on this bug as single thought per comment.  Then After you see the choices between using the bookmarks menu icon and the one i have used a placekeeper a comment would make more sense.

As I understand it, Stephen Horlander is supposed to be the person I am getting my direction from and he wants a customizable movable toolbar icon which requires both a 16x16 and a 24x24 version which the bookmarks menu icon does not have.
Comment 167 Bill Gianopoulos [:WG9s] 2010-09-25 16:12:24 PDT
(In reply to comment #159)
I am not trying to create an issue here.  It is just that my direction is to implement what was specified in comment 69 in this bug.  If that is not what is desired, I would like the UX team to get together and post a new specification so I have some idea what the desired end result is.
Comment 168 Alex Faaborg [:faaborg] (Firefox UX) 2010-09-25 20:20:19 PDT
>I would like the UX team to get together and post a new specification
>so I have some idea what the desired end result is.

In general I would like us to get this landed and we can change things like the icon / string used for the menu later.  So you can feel free to land with the current interface as soon as you have code review.

We'll discuss this in the UX meeting on monday and should have a complete spec for you then.
Comment 169 Bill Gianopoulos [:WG9s] 2010-09-26 15:13:34 PDT
(In reply to comment #168)
> >I would like the UX team to get together and post a new specification
> >so I have some idea what the desired end result is.
> 
> In general I would like us to get this landed and we can change things like the
> icon / string used for the menu later.  So you can feel free to land with the
> current interface as soon as you have code review.
> 
> We'll discuss this in the UX meeting on monday and should have a complete spec
> for you then.

Thanks for that.

My issue here is that I am way outside my comfort zone for dong this type of change.  I am a C programmer and no very little about XUL or XML bindings.  I only stepped up to do this because no one else was and I felt it was really important that Firefox $ not be released in a way that made it appear that Linux users were not being considered to be important.

So, normally if this were not so far outside my comfort level, I could probably figure out what to do about the criticisms of my approach.. My problem here is that I am really just learning what is possible to do within XUL/CSS/XML related to what is going on here, so cannot speak with any authority in trying to say why my approach might be any better that what anyone else is suggesting.
Comment 170 :Margaret Leibovic 2010-09-27 11:17:01 PDT
(In reply to comment #152)
> Anyway, in trying to prove this was not a it does not work under Linux issue,
> buy just doing a vanilla build including may previously posted patch from
> attachment 465030 [details] [diff] [review], which basically makes the linux build work as closely as
> possible to the windows version resulted in the menu still persisting in
> displaying the keyboard shortcuts.  So, it would seem that the fix in bug
> 589139 is for some reason windows only.
> 
> Anyway, I have CC'd margaret to see if we can figure out what the issue here
> is.

Are you still having this problem? I took at look at the patch I wrote in bug 589139, but I'm having trouble spotting what could cause it to break on Linux.
Comment 171 ra.ravi.rav 2010-09-28 01:52:25 PDT
I am not a developer, but a user. Not having the UI makes FF4 clunky. With FF 3.6.* I am using 'Personal Menu' addon and it gives a bit of that, it also supports ALT hide-unhide.

With the screenshots of menu with the tab bar is quite a go! As a KDE user I don't feel I want to use a different window manager just for FF.
Comment 172 Dão Gottwald [:dao] 2010-09-28 02:30:55 PDT
Bill, please don't worry about hiding the shortcut keys. We can take care of that in a follow-up bug.

Also, making the button (re)movable opens a can of worms which you'll probably want to avoid.
Comment 173 Bill Gianopoulos [:WG9s] 2010-09-28 03:10:19 PDT
Created attachment 479014 [details] [diff] [review]
WIP patch to implement Firefox menu as a toolbar item-fixes dropmarker

Still to do:

1.  Modify updateAppButtonDisplay do control the hiding/unhiding of the toolbar button.
2.  Fix the hiding of keyboard shortcuts in the Applications button menu.
Comment 174 Bill Gianopoulos [:WG9s] 2010-09-28 03:35:27 PDT
(In reply to comment #172)
> Bill, please don't worry about hiding the shortcut keys. We can take care of
> that in a follow-up bug.

OK I left this on my to-do list because I wanted to make one pore passover the winstripe theme files to see if there is code required for this that I need to copy to gnomestripe that I missed the frst time I looked.

> 
> Also, making the button (re)movable opens a can of worms which you'll probably
> want to avoid.

Yes, I have already run into this.  It is not the movable that has caused issues.  It is the possibility of removing it that necessitated replacing some "ifdef MENUBAR_CAN_AUTOHIDE" with the more klunky try/catch.

Since I currently have it re(movable) I will leave it that way until I get everything else working, and make in non (re)movable for the review ready patch.
Comment 175 Frederic Bezies 2010-09-28 09:56:58 PDT
Cannot get it build with last patch : 

nsNetscapeProfileMigratorBase.cpp
/home/fred/logs/fox/src/browser/components/migration/src/nsNetscapeProfileMigratorBase.cpp: In member function ‘nsresult nsNetscapeProfileMigratorBase::ImportNetscapeCookies(nsIFile*)’:
/home/fred/logs/fox/src/browser/components/migration/src/nsNetscapeProfileMigratorBase.cpp:391:32: attention : suggest parentheses around ‘&&’ within ‘||’
nsSeamonkeyProfileMigrator.cpp
nsPhoenixProfileMigrator.cpp
nsDogbertProfileMigrator.cpp
nsOperaProfileMigrator.cpp
nsModule.cpp
make[3]: *** [libs_tier_app] Erreur 2
make[2]: *** [tier_app] Erreur 2
make[1]: *** [default] Erreur 2
make: *** [build] Erreur 2
Comment 176 Bill Gianopoulos [:WG9s] 2010-09-30 06:39:40 PDT
Created attachment 479761 [details] [diff] [review]
WIP patch to implement Firefox menu as a toolbar item-almost there!

This implements hiding the toolbar button when the menubar is hidden and also uses a better toolbar icon.

ToDo:

Make the toolbar button non-(re)movable.
Comment 177 Bill Gianopoulos [:WG9s] 2010-09-30 06:46:00 PDT
Created attachment 479763 [details]
screenshot showing new icon

This is the new icon used by the latest WIP patch.  It is basically just an edit of the bookmarks.png icon with moire saturation and some of the official firefox orange color added.  My intent here was to create something good enough for a beta release.
Comment 178 Greg K Nicholson [:gkn] 2010-09-30 10:29:21 PDT
Why not just use the Firefox icon for the Firefox menu?

It was suggested above that the Firefox icon will already be present in the title bar, but that's not true for anyone using either Ubuntu's default theme or Gnome 3's default theme. I suspect this will cover the majority of users.

If we're being consistent about branding this as the "Firefox menu" on all OSes, using any other icon would be confusingly off-brand.
Comment 179 Bill Gianopoulos [:WG9s] 2010-09-30 13:08:54 PDT
Created attachment 479875 [details] [diff] [review]
implement Firefox menu as a non (re)movable toolbar item

Addresses all previous review comments except for the shortcuts displaying in the menu, which we agreed to handle in a followup bug.
Comment 180 Bill Gianopoulos [:WG9s] 2010-09-30 13:57:28 PDT
Created attachment 479897 [details] [diff] [review]
implement Firefox menu as a non (re)movable toolbar item v2

Only difference here is to dim the applications menu button when in customize toolbars mode to indicate that it cannot be (re)moved.
Comment 181 Frederic Bezies 2010-09-30 22:33:58 PDT
Sorry to spam, but I cannot get it build with last version of the patch. I am under Archlinux (64 bits), with gcc 4.5.1, and make 3.8.2.

AboutRedirector.cpp
Traceback (most recent call last):
  File "/home/fred/logs/fox/src/config/JarMaker.py", line 506, in <module>
    main()
  File "/home/fred/logs/fox/src/config/JarMaker.py", line 503, in main
    localedirs=options.l10n_src)
  File "/home/fred/logs/fox/src/config/JarMaker.py", line 294, in makeJars
    jardir=jardir)
  File "/home/fred/logs/fox/src/config/JarMaker.py", line 239, in makeJar
    localedirs)
  File "/home/fred/logs/fox/src/config/JarMaker.py", line 360, in processJarSection
    outHelper, jf)
  File "/home/fred/logs/fox/src/config/JarMaker.py", line 392, in _processEntryLine
    raise RuntimeError('File "%s" not found in %s' % (src, ', '.join(src_base)))
RuntimeError: File "appmenu.png" not found in /home/fred/logs/fox/src/browser/themes/gnomestripe/browser, .
make[7]: *** [libs] Erreur 1
make[6]: *** [libs] Erreur 2
make[5]: *** [libs] Erreur 2
make[4]: *** [themes_libs] Erreur 2
make[4]: *** Attente des tâches non terminées....
DirectoryProvider.cpp
nsIFeedResultService.idl
nsIWebContentConverterRegistrar.idl
nsIFeedWriter.idl
nsFeedSniffer.cpp
nsPrivateBrowsingServiceWrapper.cpp
nsISessionStartup.idl
nsISessionStore.idl
nsIShellService.idl
nsGNOMEShellService.cpp
nsIBrowserProfileMigrator.idl
nsProfileMigrator.cpp
nsIBrowserGlue.idl
nsIBrowserHandler.idl
nsBrowserProfileMigratorUtils.cpp
nsNetscapeProfileMigratorBase.cpp
nsSeamonkeyProfileMigrator.cpp
nsPhoenixProfileMigrator.cpp
/home/fred/logs/fox/src/browser/components/migration/src/nsNetscapeProfileMigratorBase.cpp: In member function ‘nsresult nsNetscapeProfileMigratorBase::ImportNetscapeCookies(nsIFile*)’:
/home/fred/logs/fox/src/browser/components/migration/src/nsNetscapeProfileMigratorBase.cpp:391:32: attention : suggest parentheses around ‘&&’ within ‘||’
nsDogbertProfileMigrator.cpp
nsOperaProfileMigrator.cpp
nsModule.cpp
make[3]: *** [libs_tier_app] Erreur 2
make[2]: *** [tier_app] Erreur 2
make[1]: *** [default] Erreur 2
make: *** [build] Erreur 2

Any idea ?

gcc -v
Utilisation des specs internes.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-unknown-linux-gnu/4.5.1/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configuré avec: ../configure --prefix=/usr --enable-languages=c,c++,fortran,objc,obj-c++,ada --enable-shared --enable-threads=posix --enable-__cxa_atexit --enable-clocale=gnu --enable-gnu-unique-object --enable-lto --enable-plugin --disable-multilib --disable-libstdcxx-pch --with-system-zlib --with-ppl --with-cloog --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man --infodir=/usr/share/info
Modèle de thread: posix
gcc version 4.5.1 (GCC)
Comment 182 Bill Gianopoulos [:WG9s] 2010-10-01 03:10:28 PDT
Created attachment 480062 [details]
appmenu.png
Comment 183 Bill Gianopoulos [:WG9s] 2010-10-01 03:13:54 PDT
(In reply to comment #181)
> Sorry to spam, but I cannot get it build with last version of the patch. I am
> under Archlinux (64 bits), with gcc 4.5.1, and make 3.8.2.

> RuntimeError: File "appmenu.png" not found in

Not sure if it is an issue with the patch or if you are applying it using a utility that does not support binary diffs.

In any event I just added an attachment containing the appmenu.png file.  If you add this file to browser/themes/gnomestripe/browser, after applying the rest of the patch, that should fix your issue.
Comment 184 Frederic Bezies 2010-10-01 03:19:13 PDT
I'm using good old patch :)

patch --version
patch 2.6.1

I have no time to verify this option before at least 6 hours from now. I will tell you.
Comment 185 Bill Gianopoulos [:WG9s] 2010-10-01 04:05:29 PDT
(In reply to comment #184)
> I'm using good old patch :)
> 
> patch --version
> patch 2.6.1
> 
> I have no time to verify this option before at least 6 hours from now. I will
> tell you.

If you use do an "hg import --no-commit" instead of using patch, the patch as attached here should work correctly and create the appmenu.png file in the correct location.  patch does not support binary diffs.
Comment 186 Frederic Bezies 2010-10-01 09:06:05 PDT
Thanks for all the infos. I will wait until final patch is available, and I agree too with comments which talk about using firefox icon instead of a new one.
Comment 187 Bill Gianopoulos [:WG9s] 2010-10-01 09:26:00 PDT
(In reply to comment #186)
> Thanks for all the infos. I will wait until final patch is available, and I
> agree too with comments which talk about using firefox icon instead of a new
> one.

The intention of this bug is to get the mechanics working correctly.  The final ICON used in the release version will be determined outside of the scope of this bug.
Comment 188 Bill Gianopoulos [:WG9s] 2010-10-01 16:17:19 PDT
You can obviously add the following to userChrome.css to make it use the Application icon:

#appmenu-button {
  list-style-image: url("chrome://branding/content/icon16.png") !important;
}
Comment 189 Bill Gianopoulos [:WG9s] 2010-10-02 08:36:12 PDT
I was looking at the proposed patches for bug 572160 and realized there is an issue with the patch for this bug in the way I have implemented it.  The issue is in misuse of MENUBAR_CAN_AUTOHIDE.

There were some feature driven ifdefs (#ifdef MENUBAR_CAN_AUTO_HIDE) that I have changed to be Platform dependent ifdefs.  In some cases this is definitely wrong.

What I should have done was defined another feature dependent (CAN_DRAW_IN_TITLEBAR) thing to use for the ifdef and use that when the code is definitely dependent on drawing in the titlebar.

In this way whatever fix eventually is decided on for bug 572160 it can depend on CAN_DRAW_IN_TITLEBAR instead of MENUBAR_CAN_AUTOHIDE or XP_WIN.

SO, I was going to rewrite my patch here to implement this.

However, I now have the question that assuming this gets landed for Firefox 4 and then a few months or a year later we get the ability to draw in the titlebar under Linux, would we:

1.  Convert Linux to use the text based not on tabbar firefox button as is now down in windows, or

2.  Leave it with the toolbar button and on the tabbar and just push the tabbar up into the titlebar?

This makes a difference as too how much of this patch I convert to not use the platform specific ifdefs.

So, I would like a UX team opinion as to which of the 2 above paths would be more likely to be followed.
Comment 190 Bill Gianopoulos [:WG9s] 2010-10-02 10:49:53 PDT
I decided to just do 2 patches, one for each of the approaches in comment 189.  You can just r- the one you rule out saying you prefer the other approach.
Comment 191 Bill Gianopoulos [:WG9s] 2010-10-02 12:17:17 PDT
Created attachment 480422 [details] [diff] [review]
comment 189 approach 1

By way of explanation, the reason I added the define of APPMENU_ON_TABBAR in browser.xul is that in .xul files we don't seem to support:

  #if defined(MENUBAR_CAN_AUTOHIDE) && !defined(CAN_DRAW_IN_TITLEBAR)

which is what I wanted to do.

Doing it with seperate ifdefs was just too convoluted to follow.
Comment 192 Bill Gianopoulos [:WG9s] 2010-10-02 12:18:38 PDT
Created attachment 480424 [details] [diff] [review]
comment 189 approach 2
Comment 193 Bill Gianopoulos [:WG9s] 2010-10-02 12:20:39 PDT
It should be noted that either approach works equally well for this bug.  The reason for the 2 patches is that if we know what the plan is for after we can draw int he titlebar and pick the appropriate patch it should save some work when it comes time to implement drawing in the titlebar.
Comment 194 Bill Gianopoulos [:WG9s] 2010-10-05 16:54:34 PDT
I should also point out that my personal opinion is that Approach 1 is the correct one.  I think that once we have the capability of drawing in the titlebar under Linux, continuing to perpetuate this divergence in what is the new look of Firefox is the correct approach form a branding perspective.

That is why the builds on my server at http://www.wg9s.com/mozilla/firefox/ include the approach 1 patch.

My only reason for including the alternative patch, was in case I was wrong and using a toolbar button for Linux was more of a Linux native look and feel decision rather than we really can't implement the windows design under Linux yet.
Comment 195 antistress 2010-10-06 11:04:03 PDT
Many thanks Bill Gianopoulos for the patches builds : it makes testing easier.
I've tested the linux 64 bits one on Ubuntu 10.10 RC (GNOME) and i'd like to point out a strange behaviour :
The arrow close to the menu entry that is used to bring the submenu is not linked to the entry itself (see screenshot attached) therefore hovering the menu entry itself with the mouse doesn't bring the submenu unless the arrow itself is hovered.

Another thing that i've noticed is that the Edit menu entry has icons close to it (scissors, clipboard...) wheareas GNOME has a setting to not show these menu icons by default (since GNOME 2.28 - see https://bugzilla.gnome.org/show_bug.cgi?id=557469)
Comment 196 antistress 2010-10-06 11:05:43 PDT
Created attachment 481273 [details]
Arrow close to a menu entry is not linked to the corresponding entry
Comment 197 antistress 2010-10-06 11:08:44 PDT
About the "arrow not linked to the corresponding entry" thing, i'd like to add that the Web Developer entry works nice unlike other entries
Comment 198 Bill Gianopoulos [:WG9s] 2010-10-06 11:43:56 PDT
(In reply to comment #195)
> Many thanks Bill Gianopoulos for the patches builds : it makes testing easier.
> I've tested the linux 64 bits one on Ubuntu 10.10 RC (GNOME) and i'd like to
> point out a strange behaviour :
> The arrow close to the menu entry that is used to bring the submenu is not
> linked to the entry itself (see screenshot attached) therefore hovering the
> menu entry itself with the mouse doesn't bring the submenu unless the arrow
> itself is hovered.

This all works just a squirrelly under Windows.  The intention here was just to get a way to access the menu and not fix issues that already exist in how the menu works once it is opened.

> 
> Another thing that i've noticed is that the Edit menu entry has icons close to
> it (scissors, clipboard...) wheareas GNOME has a setting to not show these menu
> icons by default (since GNOME 2.28 - see
> https://bugzilla.gnome.org/show_bug.cgi?id=557469)

Well the way the Edit menu item works is that its function is entirely dependent on the icons.  It has no textbased submenu, so I guess you are asking that the entire Edit menu choice not display if the user has selected not to display menu icons?  That might make sense, but I think it is another thing that is outside of the scope of this bug and would be a followup bug after this lands (assuming it actually does land).
Comment 199 Michael Monreal [:monreal] 2010-10-06 11:57:02 PDT
(In reply to comment #198)
> Well the way the Edit menu item works is that its function is entirely
> dependent on the icons.

From a GNOME POV I don't see the problem. This is no traditional UI, which only has icons in addition to labels but more like actual buttons. Chrome does the same thing, so this may very well end up as "what users expect".
Comment 200 Alex Faaborg [:faaborg] (Firefox UX) 2010-10-06 14:40:02 PDT
>>therefore hovering the
>> menu entry itself with the mouse doesn't bring the submenu unless the arrow
>> itself is hovered.
>
>This all works just a squirrelly under Windows.

we'll be fixing that with bug 589146 for both platforms
Comment 201 antistress 2010-10-06 15:30:22 PDT
in reply to comment #198 (1°) & 200 : i'm not sure to understand : that bug (having the arrow independant from the label) is known and is not Linux specific ? Even considering that the Web Developer entry already works nice ?

in reply to comment #198 (2°) & 199 : i didn't know that Chromium acted that way. Looks weird to me (why having sometimes submenu and sometimes buttons ?! there is no consistency) but this is not the place to discuss that. 
On the contrary i think that buttons should be label-only on GNOME when menus_have_icons is set to false* (like Chromium does) whereas buttons could be icon-only on the contrary

2 more things (see  former screenshot for both of them) :
When Menu bar is unticked in the View menu, the new button is shown but the the menu bar remains
The menu that is displayed from the new button has 2 pane  (but i think it's a feature of Firefox 4 ?)

Thanks

* which is default since GNOME 2.28
Comment 202 Bill Gianopoulos [:WG9s] 2010-10-07 19:03:19 PDT
Comment on attachment 480424 [details] [diff] [review]
comment 189 approach 2

This patch has bit-rottted, and I decided that I don't really have the time to keep both versions up-to-date with trunk changes.  Since I think the other approach is better, I am withdrawing this patch form review consideration.
Comment 203 Bill Gianopoulos [:WG9s] 2010-10-07 19:04:52 PDT
Created attachment 481736 [details] [diff] [review]
patch updated to tip - 2010/10/07

Update patch to current trunk.
Comment 204 Bill Gianopoulos [:WG9s] 2010-10-08 12:28:31 PDT
(In reply to comment #170)
> Are you still having this problem? I took at look at the patch I wrote in bug
> 589139, but I'm having trouble spotting what could cause it to break on Linux.

Yes, I am still having this issue, but I have done a lot more testing and I think that somehow the native Linux themeing is interfering with the selectors in browser/base/content/browser.css properly matching.

In particualr this code:

.split-menuitem-menu > .menu-accel-container {
  display: none;
}

in browser.css does not seem to be applied even though dom inspector would lead me to believe it should be.

Everything works fine if I use any third party theme.  This only fails to work if i use the defualt theme so I thkink it is some kind of issue with the native themeing code that is overriding the rules I am using in css.
Comment 205 Bill Gianopoulos [:WG9s] 2010-10-08 12:37:45 PDT
(In reply to comment #204)
> Everything works fine if I use any third party theme.  This only fails to work
> if i use the defualt theme so I thkink it is some kind of issue with the native
> themeing code that is overriding the rules I am using in css.

I should have pointed out before you ask.  None of these third party themes that I have tested with have been updated to Firefox 4. They are all Firefox 3 third party themes, so I don;t think the issue is that gnomestirpe needs some kind of appmenu support update that I missed somewhere.
Comment 206 Dão Gottwald [:dao] 2010-10-09 03:06:55 PDT
Comment on attachment 481736 [details] [diff] [review]
patch updated to tip - 2010/10/07

>+#ifdef APPMENU_ON_TABBAR
>+      <toolbaritem id="appmenu-toolbar-button-container"
>+                   class="chromeclass-toolbar-additional"
>+                   title="&brandShortName;">
>+        <toolbarbutton id="appmenu-button"
>+                       class="toolbarbutton"
>+                       type="menu"
>+                       label="&brandShortName;"

Make this &appMenuButton.label;

>+                       tooltiptext="&appMenuButton.tooltip;">
>+#include browser-appmenu.inc
>+        </toolbarbutton>
>+      </toolbaritem>

Why is the structure <toolbaritem><toolbarbutton/></toolbaritem> instead of just <toolbarbutton/>?

>+#appmenu-button.toolbarbutton {

This selector looks broken.

>+#appmenuSecondaryPane {
>+  border-left: 1px solid;

Use -moz-border-start instead for RTL support.
Comment 207 Bill Gianopoulos [:WG9s] 2010-10-09 19:41:47 PDT
(In reply to comment #206)
> Comment on attachment 481736 [details] [diff] [review]
> patch updated to tip - 2010/10/07
> 
> >+#ifdef APPMENU_ON_TABBAR
> >+      <toolbaritem id="appmenu-toolbar-button-container"
> >+                   class="chromeclass-toolbar-additional"
> >+                   title="&brandShortName;">
> >+        <toolbarbutton id="appmenu-button"
> >+                       class="toolbarbutton"
> >+                       type="menu"
> >+                       label="&brandShortName;"
> 
> Make this &appMenuButton.label;

Done!

> 
> >+                       tooltiptext="&appMenuButton.tooltip;">
> >+#include browser-appmenu.inc
> >+        </toolbarbutton>
> >+      </toolbaritem>
> 
> Why is the structure <toolbaritem><toolbarbutton/></toolbaritem> instead of
> just <toolbarbutton/>?

I did this when the toolbar button was (re)movable.  Preliminary testing indicates it is no longer required, so I removed it.
 
> >+#appmenu-button.toolbarbutton {
> 
> This selector looks broken.

I do not understand the issue here.  This is a perfectly valid CSS selector that matches an element with an identifier of appmenu-button, if and only if it also has the class toolbarbutton associated with it.  My intention here was to match the Linux appmenu button by using a rule which would not match the Windows appmenu button.
This way third party themers that copied the code form here could do something like this in their theme:

#appmenu-button-container > #appmenu-button {
  styling for windows button goes here
}
#appmenu-button.toolbarbutton {
  styling for Linux button goes here
}

In this way they could provide one theme to work with both Linux and Windows that could sperratley style this element for the 2 cases.

> >+#appmenuSecondaryPane {
> >+  border-left: 1px solid;
> 
> Use -moz-border-start instead for RTL support.

Done!

Thanks for the tip and the explanation.  I had wondered why so many places used this non-standard method of defining the border when there seemed to be a standard way to accomplish the same thing.

I will post a new patch as soon as my builds finish and I can do some testing.
Comment 208 Bill Gianopoulos [:WG9s] 2010-10-09 19:47:21 PDT
(In reply to comment #207)
> > >+#appmenu-button.toolbarbutton {
> > 
> > This selector looks broken.
> 
> I do not understand the issue here.  This is a perfectly valid CSS selector
> that matches an element with an identifier of appmenu-button, if and only if it
> also has the class toolbarbutton associated with it.  My intention here was to
> match the Linux appmenu button by using a rule which would not match the
> Windows appmenu button.
> This way third party themers that copied the code form here could do something
> like this in their theme:
> 
> #appmenu-button-container > #appmenu-button {
>   styling for windows button goes here
> }
> #appmenu-button.toolbarbutton {
>   styling for Linux button goes here
> }
> 
> In this way they could provide one theme to work with both Linux and Windows
> that could sperratley style this element for the 2 cases.

My alternative idea here was to give the button a different identifier if it is a toolbar button.  Like use appmenu-toolbar-button instead of appmenu-button.  I could do that if you prefer.
Comment 209 Bill Gianopoulos [:WG9s] 2010-10-09 22:09:13 PDT
Created attachment 482113 [details] [diff] [review]
addresses review comments

Addresses review comments.

Also, I decided, as an extension author who has an extension that modifies the Firefox menu, that any perceived advantage for extensions authors in maintaining the same identifier for the appmenu-button under Windows and Linux is far outweighed by the complications it adds to theme developers by having elements with such diverse requirements for themeing having the same identifier.

So, I have gone with the idea of changing the identifier for the applications menu under Linux as being appmenu-toolbar-button.

This also clears your review issue with the #appmenu-button.toolbarbutton CSS selector.
Comment 210 Bill Gianopoulos [:WG9s] 2010-10-10 06:23:04 PDT
Created attachment 482132 [details] [diff] [review]
addresses review comments

It appears that I accidentally attached the wrong patch file the first time.
Comment 211 Dão Gottwald [:dao] 2010-10-11 03:57:33 PDT
Comment on attachment 482132 [details] [diff] [review]
addresses review comments

>+          </hbox>
>+            <menuitem id="appmenu_downloads"

Indentation is off

>+#ifdef APPMENU_ON_TABBAR
>+      <toolbarbutton id="appmenu-toolbar-button"
>+                     class="toolbarbutton chromeclass-toolbar-additional"

Did you mean class="toolbarbutton-1 ..."?

> %ifdef XP_MACOSX
> toolbar[type="menubar"] {
>   min-height: 0 !important;
>   border: 0 !important;
> }
>-%endif
>-
>-%ifdef XP_WIN
>+%else
> toolbar[type="menubar"][autohide="true"] {
>   -moz-binding: url("chrome://global/content/bindings/toolbar.xml#toolbar-menubar-autohide");
>   overflow: hidden;
> }

You need to take autohide="true" into account in global.css for the :-moz-system-metric(menubar-drag) case.
Comment 212 Bill Gianopoulos [:WG9s] 2010-10-11 15:42:35 PDT
(In reply to comment #211)
> Comment on attachment 482132 [details] [diff] [review]
> addresses review comments
> 
> >+          </hbox>
> >+            <menuitem id="appmenu_downloads"
> 
> Indentation is off

Fixed!  (although I think I deserve extra credit here.  I just copied the bad indentation from browser.xul)

> >+#ifdef APPMENU_ON_TABBAR
> >+      <toolbarbutton id="appmenu-toolbar-button"
> >+                     class="toolbarbutton chromeclass-toolbar-additional"
> 
> Did you mean class="toolbarbutton-1 ..."?

Well these comments from browser.xul seem to indicate that the toolbarbutton-1 class is intended for (re)movable toolbar buttons only, as it seemed to indicate this class was related to the items displayed in the toolbar palette.  It is very possible that I am wrong here.  So should I change this to toolbarbutton-1 and add the identifier to primaryToolbarButtons?

    <toolbarpalette id="BrowserToolbarPalette">
 
# Update primaryToolbarButtons in browser/themes/browserShared.inc when adding
# or removing default items with the toolbarbutton-1 class. 
 
> > %ifdef XP_MACOSX
> > toolbar[type="menubar"] {
> >   min-height: 0 !important;
> >   border: 0 !important;
> > }
> >-%endif
> >-
> >-%ifdef XP_WIN
> >+%else
> > toolbar[type="menubar"][autohide="true"] {
> >   -moz-binding: url("chrome://global/content/bindings/toolbar.xml#toolbar-menubar-autohide");
> >   overflow: hidden;
> > }
> 
> You need to take autohide="true" into account in global.css for the
> :-moz-system-metric(menubar-drag) case.

Is this what you meant, or were you looking for something different?

+toolbar[type="menubar"][autohide="true"]:-moz-system-metric(menubar-drag),
 toolbar[type="menubar"]:not(-moz-lwtheme):-moz-system-metric(menubar-drag) {
   -moz-binding: url("chrome://global/content/bindings/toolbar.xml#toolbar-drag");
 }
Comment 213 Dão Gottwald [:dao] 2010-10-11 15:50:03 PDT
(In reply to comment #212)
> > >+#ifdef APPMENU_ON_TABBAR
> > >+      <toolbarbutton id="appmenu-toolbar-button"
> > >+                     class="toolbarbutton chromeclass-toolbar-additional"
> > 
> > Did you mean class="toolbarbutton-1 ..."?
> 
> Well these comments from browser.xul seem to indicate that the toolbarbutton-1
> class is intended for (re)movable toolbar buttons only, as it seemed to
> indicate this class was related to the items displayed in the toolbar palette. 
> It is very possible that I am wrong here.  So should I change this to
> toolbarbutton-1 and add the identifier to primaryToolbarButtons?

toolbarbutton-1 isn't only for removable items. Anyway, it may not make a difference here.

There's no "toolbarbutton" class, though, and I don't think you want to introduce it either.

> > You need to take autohide="true" into account in global.css for the
> > :-moz-system-metric(menubar-drag) case.
> 
> Is this what you meant, or were you looking for something different?
> 
> +toolbar[type="menubar"][autohide="true"]:-moz-system-metric(menubar-drag),
>  toolbar[type="menubar"]:not(-moz-lwtheme):-moz-system-metric(menubar-drag) {
>    -moz-binding:
> url("chrome://global/content/bindings/toolbar.xml#toolbar-drag");
>  }

The toolbar-drag binding must not be applied if the menubar is supposed to use the autohide binding.
Comment 214 Bill Gianopoulos [:WG9s] 2010-10-11 16:53:53 PDT
(In reply to comment #213)
> toolbarbutton-1 isn't only for removable items. Anyway, it may not make a
> difference here.
> 
> There's no "toolbarbutton" class, though, and I don't think you want to
> introduce it either.

I could have sworn that I did a serach and found another non-movable button that had a class of toolbarbutton and copied. I sure can;t find it now.  maybe I was dreaming.  So I changed it to toolbarbutton-1.
 
> The toolbar-drag binding must not be applied if the menubar is supposed to use
> the autohide binding.

OK so more like this then:

toolbar[type="menubar"]:not([autohide="true"]):not(-moz-lwtheme):-moz-system-metric(menubar-drag) {
  -moz-binding: url("chrome://global/content/bindings/toolbar.xml#toolbar-drag");
}

Running new builds and will attach a new patch as soon as I can test.
Comment 215 Bill Gianopoulos [:WG9s] 2010-10-11 18:46:09 PDT
Created attachment 482427 [details] [diff] [review]
patch v5
Comment 216 Dão Gottwald [:dao] 2010-10-12 02:39:58 PDT
Comment on attachment 482427 [details] [diff] [review]
patch v5

this needs an update for bug 602662
Comment 217 Bill Gianopoulos [:WG9s] 2010-10-12 16:33:30 PDT
(In reply to comment #216)
> Comment on attachment 482427 [details] [diff] [review]
> patch v5
> 
> this needs an update for bug 602662

Thanks for the heads-up on this!

This saved me the effort of kicking off my builds before I left for work just have them fail and also in trying to figure out where the conflict actually was.

New patch is forthcoming.
Comment 218 Bill Gianopoulos [:WG9s] 2010-10-12 21:01:32 PDT
Created attachment 482773 [details] [diff] [review]
patch v6
Comment 219 Dão Gottwald [:dao] 2010-10-13 01:34:05 PDT
Comment on attachment 482773 [details] [diff] [review]
patch v6

>+#appmenuSecondaryPane {
>+  -moz-border-start: 1px solid;

You should specify a color here, e.g. ThreeDShadow.
Comment 220 Bill Gianopoulos [:WG9s] 2010-10-13 05:51:07 PDT
(In reply to comment #219)
> Comment on attachment 482773 [details] [diff] [review]
> patch v6
> 
> >+#appmenuSecondaryPane {
> >+  -moz-border-start: 1px solid;
> 
> You should specify a color here, e.g. ThreeDShadow.

I tried ThreeDShadow, and the problem I ran into was that with a dark OS theme, the border became invisible (I guess this could be an issue with the theme I was using).  Anyway, MenuText seemed to work, but I got scared away from that by the Not all system colors are supported on all systems disclaimer.

Would it be acceptable to you for me to use -moz-MenuBarText as the color here?
Comment 221 Michael Monreal [:monreal] 2010-10-13 07:10:49 PDT
(In reply to comment #220)
> Would it be acceptable to you for me to use -moz-MenuBarText 
> as the color here?

In most cases MenuBarText is black, right? In "normal" themes, this looks bad IMHO. ThreeDShadow is used for the menu border and horizontal separators was well, right? If so, why care for the vertical separator?

If there is a problem with dark themes, it is how the ThreeDShadow color is calculated.
Comment 222 Michael Monreal [:monreal] 2010-10-13 07:17:15 PDT
Another thing: why does the menu list "Options..." at the top level and "Preferences" in the submenu?

- differently named menu items both open the same window
- the window is called "Firefox Preferences" (matches GNOME HIG)
- ellipsis on "Options..." is wrong according to GNOME HIG

IMHO this is quite confusing. The problem probably doesn't exist on windows, where the prefs window is called "Options" and other ellipis guidelines apply
Comment 223 Bill Gianopoulos [:WG9s] 2010-10-13 07:34:02 PDT
(In reply to comment #221)
> (In reply to comment #220)
> > Would it be acceptable to you for me to use -moz-MenuBarText 
> > as the color here?
> 
> In most cases MenuBarText is black, right? In "normal" themes, this looks bad
> IMHO. ThreeDShadow is used for the menu border and horizontal separators was
> well, right? If so, why care for the vertical separator?
> 
> If there is a problem with dark themes, it is how the ThreeDShadow color is
> calculated.

OK I looked at how the separator is done.  it is actually a ThreeDShadow adjacent to a ThreeDHighlight.  I guess the theory is that one or the other will be visible.  I can do that.
Comment 224 Bill Gianopoulos [:WG9s] 2010-10-13 07:41:30 PDT
(In reply to comment #222)
> Another thing: why does the menu list "Options..." at the top level and
> "Preferences" in the submenu?
> 
> - differently named menu items both open the same window
> - the window is called "Firefox Preferences" (matches GNOME HIG)

I will fix this.

> - ellipsis on "Options..." is wrong according to GNOME HIG
> 
> IMHO this is quite confusing. The problem probably doesn't exist on windows,
> where the prefs window is called "Options" and other ellipis guidelines apply

I can remove the elipsis also I guess, but I actually think that on both Windows and Linux the elipsis is wrong on many of these menus.  Seems to me there should either be an elipsis or the arrow.  Having both just seems redundant.

I will attach a new patch and ask for a second review from you.
Comment 225 Michael Monreal [:monreal] 2010-10-13 07:48:09 PDT
(In reply to comment #224)
> I can remove the elipsis also I guess, but I actually think that on both
> Windows and Linux the elipsis is wrong on many of these menus.  Seems to me
> there should either be an elipsis or the arrow.  Having both just seems
> redundant.

The problem here is that traditionally, a menu item is either some kind of action or a category name (headline for the submenu). The new firefox menu breaks with this tradition. Still, the GNOME HIG is clear I think:

---
Label the menu item with a trailing ellipsis ("...") only if the command requires further input from the user before it can be performed. Do not add an ellipsis to items that only present a confirmation dialog (such as Delete), or that do not require further input (such as Properties, Preferences or About).
---

My first impression was that "Options..." has the ellipsis because it is also a sub menu, but this is not true because "New Tab" would have ellipsis too in that case.
Comment 226 Bill Gianopoulos [:WG9s] 2010-10-13 08:27:41 PDT
Created attachment 482857 [details] [diff] [review]
patch v7 - addresses Michael Monreal's issues

This addresses the border and Preferences menu item issues.

Carrying Dão's r+ forward.
Comment 227 Dão Gottwald [:dao] 2010-10-13 08:50:47 PDT
(In reply to comment #226)
> Carrying Dão's r+ forward.

Don't do that when making changes previously not discussed...
Comment 228 Michael Monreal [:monreal] 2010-10-13 08:51:49 PDT
Created attachment 482867 [details]
v7 screenshot

(In reply to comment #223)
> OK I looked at how the separator is done.  it is actually a ThreeDShadow
> adjacent to a ThreeDHighlight.

The separators in the new main menu do not seem to have ThreeDHighlight? With this, v7 now looks bad on the top and bottom line, because the separator draws all the way to the border (-> screenshot). I would just go with a normal ThreeDShadow for now and get this committed. Maybe clean up some cosmetic issues later.
Comment 229 Bill Gianopoulos [:WG9s] 2010-10-13 09:58:38 PDT
(In reply to comment #228)
> Created attachment 482867 [details]
> v7 screenshot
> 
> (In reply to comment #223)
> > OK I looked at how the separator is done.  it is actually a ThreeDShadow
> > adjacent to a ThreeDHighlight.
> 
> The separators in the new main menu do not seem to have ThreeDHighlight? With
> this, v7 now looks bad on the top and bottom line, because the separator draws
> all the way to the border (-> screenshot). I would just go with a normal
> ThreeDShadow for now and get this committed. Maybe clean up some cosmetic
> issues later.

OK.  I'll go back to just the ThreeDShadow and if there is really an issue with dark themes that can be worked later.  This actually looks much better on my system than it does on yours, which is part of the issue with trying to customize the look of something that depends partially on the native themeing I guess.
Comment 230 Michael Monreal [:monreal] 2010-10-13 10:07:24 PDT
(In reply to comment #229)
> This actually looks much better on my system than it does on yours

Sure... I selected this color scheme to become aware of exactly this kind of problems in Fx :)
Comment 231 Bill Gianopoulos [:WG9s] 2010-10-13 11:16:33 PDT
Created attachment 482913 [details] [diff] [review]
patch v8

OK.  THis si what Dao R+'d before with just the change from "Options..." to "Preferences" in the Menu.

Not sure this really needs a new r+ but asking anyway.
Comment 232 Bill Gianopoulos [:WG9s] 2010-10-13 11:26:26 PDT
(In reply to comment #230)
> (In reply to comment #229)
> > This actually looks much better on my system than it does on yours
> 
> Sure... I selected this color scheme to become aware of exactly this kind of
> problems in Fx :)

Actually, it is more that with the theme I am using, the things that look bad at the intersection, do not actually intersect.
Comment 233 Bill Gianopoulos [:WG9s] 2010-10-13 11:44:18 PDT
(In reply to comment #227)
> (In reply to comment #226)
> > Carrying Dão's r+ forward.
> 
> Don't do that when making changes previously not discussed...

OK. Sorry.
Comment 234 Bill Gianopoulos [:WG9s] 2010-10-13 12:53:01 PDT
Comment on attachment 482913 [details] [diff] [review]
patch v8

Asking for approval2.0 or approval for beta8, whatever is being done now.

I would like this to be checked in soon to maximize the amount of trunk bake time before this gets before it is incorporated into a beta or release candidate.
Comment 235 Bill Gianopoulos [:WG9s] 2010-10-13 13:02:50 PDT
(In reply to comment #233)
> (In reply to comment #227)
> > (In reply to comment #226)
> > > Carrying Dão's r+ forward.
> > 
> > Don't do that when making changes previously not discussed...
> 
> OK. Sorry.

Actually Dão, I would like to thank you for the review.  I learned a lot here, which I think is one of the points of the review process.  Besides making sure people don't check in things without peer review, I think the idea is that people trying to work on an area they are not familiar with submitting a patch and asking for a review from the responsible people should end up learning more about an area of the codebase that you they not be familiar with.

Also, thank you for being patient with me as someone trying to learn here.
Comment 236 Dão Gottwald [:dao] 2010-10-15 07:51:27 PDT
I filed bug 604650 and bug 604651 on the accelerator text and the icon.
Comment 237 Mike Beltzner [:beltzner, not reading bugmail] 2010-10-18 14:30:04 PDT
Comment on attachment 482913 [details] [diff] [review]
patch v8

This needs ui+r before approval. I bet screenshots would help with that.
Comment 238 Bill Gianopoulos [:WG9s] 2010-10-18 15:48:41 PDT
Created attachment 484130 [details]
screenshot showing application menu in open state

This shows the menu in the open state.
Comment 239 Bill Gianopoulos [:WG9s] 2010-10-18 15:53:44 PDT
It is important to note that for the purpose of this bug, the UI review is not really to make sure this is in the final release state, just to make sure that what we have here is OK for beta8.  Bug 604650 is intended to select the proper toolbar icon and bug 604651 is still pending to hide the accelerator shortcuts from the top level applications menu.  I have a patch on this that is dependent on the patch here.  I will be posting a screenshot of what the menu looks like with that patch applied.

Also this screenshot was taken on a system with the gnome default of not displaying the menu icons being overridden to show what the menu looks like with the icons.  If you don't override that gnome setting this displays, as expected, with no menu icons.
Comment 240 antistress 2010-10-18 16:49:48 PDT
I've tried latest build from Bill Gianopoulos server on Ubuntu 10.10 :
nice work but i've noticed 2 things :
- Web Developer entry still behave differently from other entries (regarding the fact pointing the label displays the submenu (note that i much prefer this behaviour than the other)
- i'd like to see my c#201 discussed, concerning the cut/copy/paste icons that are displayed even with GNOME is set to not show these menu
icons by default. On Chromium, these buttons are label-only on GNOME when
menus_have_icons is set to false. I think these buttons should have 2 state : label-only on GNOME when menus_have_icons is set to false and icon-only on the contrary. At present time icon-only is the only implementation which seems really weird considering that no other GNOME application use these icons by default. Thanks
Comment 241 Yann Brelière 2010-10-19 01:47:55 PDT
(In reply to comment #240)
> - i'd like to see my c#201 discussed, concerning the cut/copy/paste icons that
> are displayed even with GNOME is set to not show these menu
> icons by default. On Chromium, these buttons are label-only on GNOME when
> menus_have_icons is set to false. I think these buttons should have 2 state :
> label-only on GNOME when menus_have_icons is set to false and icon-only on the
> contrary. At present time icon-only is the only implementation which seems
> really weird considering that no other GNOME application use these icons by
> default. Thanks

No other GNOME app use these icons? Open gedit, for example. Look at the toolbar. Notice the icons. These are buttons. Even on this Firefox "menu", the copy/cut/paste icons are buttons, not menu entries with an icon (or not) illustrating the menu entry.
Comment 242 antistress 2010-10-19 02:12:33 PDT
(In reply to comment #241)
> (In reply to comment #240)
> > - i'd like to see my c#201 discussed, concerning the cut/copy/paste icons that
> > are displayed even with GNOME is set to not show these menu
> > icons by default. On Chromium, these buttons are label-only on GNOME when
> > menus_have_icons is set to false. I think these buttons should have 2 state :
> > label-only on GNOME when menus_have_icons is set to false and icon-only on the
> > contrary. At present time icon-only is the only implementation which seems
> > really weird considering that no other GNOME application use these icons by
> > default. Thanks
> 
> No other GNOME app use these icons? Open gedit, for example. Look at the
> toolbar. Notice the icons. These are buttons. Even on this Firefox "menu", the
> copy/cut/paste icons are buttons, not menu entries with an icon (or not)
> illustrating the menu entry.

You can't take these exemples since none of these applications makes use of these "buttons within menu" that can be find in current patch. 
From a user point of view, everything that sit in a menu is a menu entry. It can be a label-only entry, an icon-only entry or an icon+label entry. 
When GNOME has menus_have_icons set to false, you'll not find any application displaying cut/copy/paste icons within menus, only labels.
Comment 243 antistress 2010-10-19 02:24:57 PDT
For the record, Bug 465669 was about considering scaling back the use of menu icons in the Tango theme
Comment 244 Bill Gianopoulos [:WG9s] 2010-10-19 05:28:28 PDT
(In reply to comment #240)
> I've tried latest build from Bill Gianopoulos server on Ubuntu 10.10 :
> nice work but i've noticed 2 things :
> - Web Developer entry still behave differently from other entries (regarding
> the fact pointing the label displays the submenu (note that i much prefer this
> behaviour than the other)

This is being worked in bug 589146.

> - i'd like to see my c#201 discussed, concerning the cut/copy/paste icons that
> are displayed even with GNOME is set to not show these menu
> icons by default. On Chromium, these buttons are label-only on GNOME when
> menus_have_icons is set to false. I think these buttons should have 2 state :
> label-only on GNOME when menus_have_icons is set to false and icon-only on the
> contrary. At present time icon-only is the only implementation which seems
> really weird considering that no other GNOME application use these icons by
> default. Thanks

I would rather not do any more changes than absolutely necessary to the menu itself until after this lands on the trunk.  The most difficult part of keeping this patch up to date is syncing the new include file with changes being made in other bugs to the Firefox menu.  Every change I make to the include file in this bug just increases the work involved in keeping it in sync with other menu changes.

Also, once this lands on the trunk, it will be easier for a wider audience to see how this works and intelligently comment on what you are talking about.

So, my preference would be to do a followup bug on this issue.

Personally, I don't like the way the edit buttons work even under windows.  If they are going to be buttons, the probably belong on the titlebar next to the Firefox button.  Having them in the menu but not work at all like like menu items and having the word edit grayed out always just makes no sense.
Comment 245 Bill Gianopoulos [:WG9s] 2010-10-19 05:57:41 PDT
(In reply to comment #240)
> I've tried latest build from Bill Gianopoulos server on Ubuntu 10.10 :
> nice work but i've noticed 2 things :
> - Web Developer entry still behave differently from other entries (regarding
> the fact pointing the label displays the submenu (note that i much prefer this
> behaviour than the other)

I should have mentioned that the reason for the difference int he way the hover works, is that on the menu items where the label and the arrow highlight separately is because clicking on the label has a different action than clicking on the arrow.  The reason that WebDeveloper hover works differently is that there is no special action for clicking on the label, hence it performs the same function as clicking on the arrow and so the highlight on hover just highlights both.
Comment 246 antistress 2010-10-19 07:03:06 PDT
(In reply to comment #245)
> (In reply to comment #240)
> > I've tried latest build from Bill Gianopoulos server on Ubuntu 10.10 :
> > nice work but i've noticed 2 things :
> > - Web Developer entry still behave differently from other entries (regarding
> > the fact pointing the label displays the submenu (note that i much prefer this
> > behaviour than the other)
> 
> I should have mentioned that the reason for the difference int he way the hover
> works, is that on the menu items where the label and the arrow highlight
> separately is because clicking on the label has a different action than
> clicking on the arrow.  The reason that WebDeveloper hover works differently is
> that there is no special action for clicking on the label, hence it performs
> the same function as clicking on the arrow and so the highlight on hover just
> highlights both.

Thanks for the precision. However don't you think that this kind of behaviour may be too subtle for the vast majority of users ? (I'ts probably beyond the scope of this bug)
Hopefully menu bar will not be hidden by default on Linux and will remain an (useful - especially for notebooks) advanced feature
Comment 247 Bill Gianopoulos [:WG9s] 2010-10-19 07:12:14 PDT
(In reply to comment #246)
> (In reply to comment #245)
> > (In reply to comment #240)
> > > I've tried latest build from Bill Gianopoulos server on Ubuntu 10.10 :
> > > nice work but i've noticed 2 things :
> > > - Web Developer entry still behave differently from other entries (regarding
> > > the fact pointing the label displays the submenu (note that i much prefer this
> > > behaviour than the other)
> > 
> > I should have mentioned that the reason for the difference int he way the hover
> > works, is that on the menu items where the label and the arrow highlight
> > separately is because clicking on the label has a different action than
> > clicking on the arrow.  The reason that WebDeveloper hover works differently is
> > that there is no special action for clicking on the label, hence it performs
> > the same function as clicking on the arrow and so the highlight on hover just
> > highlights both.
> 
> Thanks for the precision. However don't you think that this kind of behaviour
> may be too subtle for the vast majority of users ? (I'ts probably beyond the
> scope of this bug)
> Hopefully menu bar will not be hidden by default on Linux and will remain an
> (useful - especially for notebooks) advanced feature

It is beyond the scope of this bug as this is the identical behavior as under windows.  As I said above, the way this works is being redesigned and addressed in bug 589146.
Comment 248 Bill Gianopoulos [:WG9s] 2010-10-19 15:48:17 PDT
(In reply to comment #240)
> - i'd like to see my c#201 discussed, concerning the cut/copy/paste icons that
> are displayed even with GNOME is set to not show these menu
> icons by default. On Chromium, these buttons are label-only on GNOME when
> menus_have_icons is set to false.

Chromium is a bad example it displays no menu icons and these buttons are label only even if menus_have_icons is set to true.

<soapbox>It should also be noted, if you actually read the bug that created the gnome change, this was not made a a design decision that menus should not have icon, it was in fact, a performance improvement suggestion that since menus open faster without icons, then menu icons should not be displayed to speed up the opening of the menu.  I would have though at least some attempt could have been made to actually fix the performance issue in displaying menu icons before copping out and just not showing them and pretending this was a design looks better decision.</soapbox>
Comment 249 Alex Faaborg [:faaborg] (Firefox UX) 2010-10-19 17:45:48 PDT
Comment on attachment 482913 [details] [diff] [review]
patch v8

A few small changes to bring this more in line with the design of the Firefox menu on other platforms:

-The button should either contain a Firefox icon or the string "Firefox"  We want this to be consistently known as "The Firefox Button."  Adding a caveat of "or a small menu icon if you are on Linux" to support documents is clunky.  Also, the commands follow the normal menu effect of building through a sentence through the path: "Firefox Print" "Firefox Find" (kind of like giving commands to a dog)

-Keyboard shortcuts should be removed from the top level.  They are available with tooltips, and on first item of the second level (which is intentionally redundant partly to expose the keyboard shortcut).  The additional complexity of displaying the keyboard shortcuts on the top level reduces the relative discoverabilty of each item, because there is more information being shown.

-The right pane of the menu should contain a different background color to differentiate it from the left pane.  Even if the user doesn't conceptualize that the left pane contains actions and the right pane contains places (new tabs and windows), the different styling will help them leverage spatial memory (bookmarks on the darker area).  Just using the window color for the right pane is fine, since on linux we are limited to extracted system colors.

-There should be some space in between downloads and add-ons to visually convey that there are two groups of commands on the right side (each containing three items).
Comment 250 Alex Faaborg [:faaborg] (Firefox UX) 2010-10-19 17:48:03 PDT
Alternatively we could address those points in follow up bugs and land this patch, really whatever people prefer.
Comment 251 Alex Faaborg [:faaborg] (Firefox UX) 2010-10-19 18:10:12 PDT
>this was not made a a design decision that menus should not have
>icon, it was in fact, a performance improvement suggestion that since menus
>open faster without icons

There's a human performance aspect here as well, if you only display icons for the highly used items, they are easier for the user to spot (compared to using icons for all of the items), so users are more likely to be able to quickly find the target they are looking for.  For instance on windows the context menu of the recycle bin places an icon next to "empty recycle bin" but not on any of the other items in the menu.  This visual search optimization is why we only included icons for some of the items in the Firefox menu.
Comment 252 Bill Gianopoulos [:WG9s] 2010-10-19 18:33:29 PDT
(In reply to comment #249)
> Comment on attachment 482913 [details] [diff] [review]
> patch v8
> 
> A few small changes to bring this more in line with the design of the Firefox
> menu on other platforms:
> 
> -The button should either contain a Firefox icon or the string "Firefox"  We
> want this to be consistently known as "The Firefox Button." 

That is what I wanted to do int he first plcae.  I can easily change to using the application icon and to using brandShortname as the button label, but Dao seemed to be unwilling to give me an r+ with that.


> -Keyboard shortcuts should be removed from the top level.  They are available
> with tooltips, and on first item of the second level (which is intentionally
> redundant partly to expose the keyboard shortcut).  The additional complexity
> of displaying the keyboard shortcuts on the top level reduces the relative
> discoverabilty of each item, because there is more information being shown.

It appears this is very difficult to do because of some interaction that I do not understand between either trying to do this via the current bindings or by trying to add a new class on the elements and do it via css and Linux native themeing that seems to somehow break both of these approaches.

> -The right pane of the menu should contain a different background color to
> differentiate it from the left pane.  Even if the user doesn't conceptualize
> that the left pane contains actions and the right pane contains places (new
> tabs and windows), the different styling will help them leverage spatial memory
> (bookmarks on the darker area).  Just using the window color for the right pane
> is fine, since on linux we are limited to extracted system colors.
> 

I tried using a different color for the righthand pane and the issue was that it did not work if i used a gnome theme that was dark rather than light.  I thought this was perhaps an issue that could be resoved by the linux theme team after this landed or was hoping someone could give me a clue as to how to make this work better.


> -There should be some space in between downloads and add-ons to visually convey
> that there are two groups of commands on the right side (each containing three
> items).

I should be able to do something here given that it does display that way under windows.  However, I might ask the question, if everywhere else where we make this type of distinction, we use a separator, why do we not do so here?
Comment 253 Alex Faaborg [:faaborg] (Firefox UX) 2010-10-19 18:58:20 PDT
>That is what I wanted to do int he first plcae.  I can easily change to using
>the application icon and to using brandShortname as the button label, but Dao
>seemed to be unwilling to give me an r+ with that.

right, vaguely remember my previous comments higher up in the bug.  In that case we should have beltzner make the final call on the button name.

>It appears this is very difficult to do because of some interaction that I do
>not understand between either trying to do this via the current bindings or by
>trying to add a new class on the elements and do it via css and Linux native
>themeing that seems to somehow break both of these approaches.

Let's move that to a follow up bug then.

>I tried using a different color for the righthand pane and the issue was that
>it did not work if i used a gnome theme that was dark rather than light.  I
>thought this was perhaps an issue that could be resoved by the linux theme team
>after this landed or was hoping someone could give me a clue as to how to make
>this work better.

Shouldn't text color always work on window color?

>I should be able to do something here given that it does display that way under
>windows.  However, I might ask the question, if everywhere else where we make
>this type of distinction, we use a separator, why do we not do so here?

Two reasons (but neither of them are really strong reasons).  We wanted the set of items to visually group together since they are all "places" while still providing some separation to reduce visual scan time (although really just the background color is probably enough to achieve that).  The second reason is that the line would have a strange appearance when viewed with the items on the left side.  Due to the padding that the lines get it wouldn't really line up with anything land mid command and would feel unbalanced.
Comment 254 Bill Gianopoulos [:WG9s] 2010-10-19 19:30:12 PDT
(In reply to comment #253)
> >That is what I wanted to do int he first plcae.  I can easily change to using
> >the application icon and to using brandShortname as the button label, but Dao
> >seemed to be unwilling to give me an r+ with that.
> 
> right, vaguely remember my previous comments higher up in the bug.  In that
> case we should have beltzner make the final call on the button name.

On the button name, it seems to me that the argument to change form using brandShortname to Menu was a holdover from when we were trying to make it be a (re)movable item.  When it was in the toolbar palette it seemed to make no sense to have it's label be Firefox or Minefield as this did not seem to indicate  the function of the button so Menu made a lot more sense.  NOw that it is no longer (re)movable it would seem it woudl make more sense to revert to it being labeled as brandShortname, the same as under Windows.

As far as the icon goes, I always wanted to use the application icon, but some people thought it looked redundant because under most Linux windows managers with tabs-on-top it would be positioned directly under the application icon in the titlebar.

> 
> >It appears this is very difficult to do because of some interaction that I do
> >not understand between either trying to do this via the current bindings or by
> >trying to add a new class on the elements and do it via css and Linux native
> >themeing that seems to somehow break both of these approaches.
> 
> Let's move that to a follow up bug then.
> 
> >I tried using a different color for the righthand pane and the issue was that
> >it did not work if i used a gnome theme that was dark rather than light.  I
> >thought this was perhaps an issue that could be resoved by the linux theme team
> >after this landed or was hoping someone could give me a clue as to how to make
> >this work better.
> 
> Shouldn't text color always work on window color?
> 

I will attach screenshots of what this ens up looking like using the "high Contrast Inverse" gnome theme which is one of the ones installed by default under fedora.

 
> >I should be able to do something here given that it does display that way under
> >windows.  However, I might ask the question, if everywhere else where we make
> >this type of distinction, we use a separator, why do we not do so here?
> 
> Two reasons (but neither of them are really strong reasons).  We wanted the set
> of items to visually group together since they are all "places" while still
> providing some separation to reduce visual scan time (although really just the
> background color is probably enough to achieve that).  The second reason is
> that the line would have a strange appearance when viewed with the items on the
> left side.  Due to the padding that the lines get it wouldn't really line up
> with anything land mid command and would feel unbalanced.

OK New patch coming up that fixes the Secondary panel spacing issue.
Comment 255 Bill Gianopoulos [:WG9s] 2010-10-19 20:08:53 PDT
(In reply to comment #250)
> Alternatively we could address those points in follow up bugs and land this
> patch, really whatever people prefer.

Here are my reasons why this should be checked-in ASAP.

1.  On the assumption that this will be in Firefox 4, I think the state it is in in is good enough to get it to the Linux UI team can massage it to a final state, and getting it in now gives more time for user feedback on any issues it might cause.

2.  I really think we need to fix the issue of MENUBAR-CAN_AUTOHIDE being overloaded to also mean CAN_DRAW_IN_TITLEBAR.  This patch separates the 2 into separate defines so that code that depends on each can be correctly ifdef'd.

3.  (largely selfish) It is a pain to keep this bug from bit-rotting because of changes being made to the Applications menu.

The overriding negative to checking it in now, is that if the issue of not being able to hide Keyboard shortcuts from the top level is a showstopper for getting this into Firefox 4, then based on my attempts to do this so far, i am not sure getting that done in time is a reasonable target.  So if that is really a showstopper, we might be at an impasse for getting this landed quickly.
Comment 256 Bill Gianopoulos [:WG9s] 2010-10-19 20:55:27 PDT
Created attachment 484610 [details] [diff] [review]
patch v9-fix spacing of Secondary pane menu items

This addresses the spacing issue on the secondary menu pane.
Comment 257 Bill Gianopoulos [:WG9s] 2010-10-19 20:56:42 PDT
Created attachment 484611 [details]
screenshot with menu open - patch v9
Comment 258 Alex Faaborg [:faaborg] (Firefox UX) 2010-10-19 20:56:57 PDT
Comment on attachment 484610 [details] [diff] [review]
patch v9-fix spacing of Secondary pane menu items

giving a ui-review+ to get this landed, with the assumption that we'll work on a few additional aspects of it in follow up bugs.
Comment 259 Bill Gianopoulos [:WG9s] 2010-10-19 20:57:57 PDT
Created attachment 484612 [details]
screenshot with menu open - patch v9 - inv theme

This is with the High Contrast Inverse Theme
Comment 260 Bill Gianopoulos [:WG9s] 2010-10-19 21:06:58 PDT
Created attachment 484617 [details]
screenshot with menu open - patch v9 - with windows color for secondary pane
Comment 261 Bill Gianopoulos [:WG9s] 2010-10-19 21:10:07 PDT
Created attachment 484618 [details]
screenshot with menu open - patch v9 - with windows color inv theme

This is with specifying the same background color as we use in winstripe but using the High Contrast Inverse Gnome Theme.

Obviously I could make the text etc. be visible by specifying test and highlight colors, etc., but this still would never fit in with the OS theme.  That is why I left it without specifying the background color in my original patch.
Comment 262 Bill Gianopoulos [:WG9s] 2010-10-19 21:17:37 PDT
OK carrying forward Dão's review, I am once again seeking approval-2.0 to try to get this landed soon for the following reasons:

1.  On the assumption that this will be in Firefox 4, I think the state it is
in in is good enough to get it to the Linux UI team can massage it to a final
state, and getting it in now gives more time for user feedback on any issues it
might cause.

2.  I really think we need to fix the issue of MENUBAR-CAN_AUTOHIDE being
overloaded to also mean CAN_DRAW_IN_TITLEBAR.  This patch separates the 2 into
separate defines so that code that depends on each can be correctly ifdef'd.

3.  (largely selfish) It is a pain to keep this bug from bit-rotting because of
changes being made to the Applications menu.
Comment 263 Mike Beltzner [:beltzner, not reading bugmail] 2010-10-20 07:01:51 PDT
Alex: you're really OK with adding this button on Linux and Linux only? Seems a little awkward to me, really, and I had always thought that we'd decided not to implement the Firefox Button on Linux and OSX.

Pretty sure I said as much earlier, and while I don't mean to spite the work, I'm still not sure I understand why we want this. Is the assumption that since it's a toolbar button that users have to manually customize that we should just let it in?
Comment 264 antistress 2010-10-20 09:25:36 PDT
(In reply to comment #263)
> Alex: you're really OK with adding this button on Linux and Linux only? Seems a
> little awkward to me, really, and I had always thought that we'd decided not to
> implement the Firefox Button on Linux and OSX.
> 
> Pretty sure I said as much earlier, and while I don't mean to spite the work,
> I'm still not sure I understand why we want this. Is the assumption that since
> it's a toolbar button that users have to manually customize that we should just
> let it in?

I'm only a user but i'd like to give some reasons in favor of having such a button.
1°) According to Bill Gianopoulos, Linux users really expects not behing left behind ; they expect to benefit from some UI changes too. The Firefox button is one of them.
2°) Firefox 4 will left the status bar, and having that button would allow to save twice the vertical space which is important on a notebook with a 7 or 9" screen : notebook users will appreciate. Moreover some Linux vendors may appreciate having such an option to keep using Firefox even if i can't speak for them : i know that Canonical was holding a discussion about switching to Chromium for its netbook version for instance
3°) If I've got it right, hiding the menu bar in favor of this button will not be default setting therefore i see some real benefits and no drawbacks
Thanks
Comment 265 Mike Beltzner [:beltzner, not reading bugmail] 2010-10-20 09:50:43 PDT
(In reply to comment #264)
> 1°) According to Bill Gianopoulos, Linux users really expects not behing left
> behind ; they expect to benefit from some UI changes too. The Firefox button 
> is one of them.

I know what Bill thinks. I don't think this is giving them the benefit of the UI changes. The Firefox Button is about removing the menubar and replacing that UI space with a button that replaces it. That's not what this patch does. It adds a button to the toolbar. We don't do that on any other operating system for a reason: doing so removes space from the tabstrip.

> 2°) Firefox 4 will left the status bar, and having that button would allow to
> save twice the vertical space which is important on a notebook with a 7 or 9"
> screen : notebook users will appreciate. Moreover some Linux vendors may
> appreciate having such an option to keep using Firefox even if i can't speak
> for them : i know that Canonical was holding a discussion about switching to
> Chromium for its netbook version for instance

Irrelevant. If distributors want to make a modification like this, they can and do know how to get in touch with us to make such modifications.

> 3°) If I've got it right, hiding the menu bar in favor of this button will not
> be default setting therefore i see some real benefits and no drawbacks
> Thanks

Adding code always has drawbacks. It must be maintained, it must be designed, and it must be tested.
Comment 266 Dão Gottwald [:dao] 2010-10-20 11:54:40 PDT
(In reply to comment #263)
> Pretty sure I said as much earlier, and while I don't mean to spite the work,
> I'm still not sure I understand why we want this. Is the assumption that since
> it's a toolbar button that users have to manually customize that we should just
> let it in?

Users wouldn't customize the button. They would hide the menu bar and the button would appear.
Comment 267 Mike Beltzner [:beltzner, not reading bugmail] 2010-10-20 12:50:24 PDT
(In reply to comment #266)
> Users wouldn't customize the button. They would hide the menu bar and the
> button would appear.

Yeah, then I'm pretty sure that we don't want this in the tabstrip. The design is that the menuButton goes in the toolbar.

Alex: does this make you reconsider, or do you feel that I'm thinking about this wrong?
Comment 268 Alex Faaborg [:faaborg] (Firefox UX) 2010-10-20 16:36:59 PDT
Trying to balance cross platform UI consistency while also meshing with the interactive design with the surrounding platform is in many cases (like this one) simply not possible.  There are however a lot of reasons why users would want to customize Firefox on one platform to have the same interactive design as Firefox on another platform.  For instance, the user might have a Windows 7 machine at work, and a linux machine at home, and want to have the two applications behave the same.  So our approach has been to fracture the Firefox UI across platforms for the sake of interactive integration on each, but still allow users to customize it to be the same everywhere.  This is also why we would like to add the Firefox button as an optional control on OS X (and already provide the option on XP).

The additional benefit is that users can save some vertical screen space with the Firefox button, so they may want to use it even if they aren't familiar with the interface from using a Windows machine.  Or the may prefer it because it makes the main window less visually complex.

>Yeah, then I'm pretty sure that we don't want this in the tabstrip. The design
>is that the menuButton goes in the toolbar.

Placing in the tabstrip isn't ideal, but without the ability to draw in the title bar this was the only way for us to expose an app level control at the right level of the interface.  We could however just ignore that for the time being and put it in the current tab's navigation toolbar just to free up some tabstrip space.  I'm mostly indifferent with a slight preference towards placing it at the right level at the cost of taking some tabstrip space.
Comment 269 Bill Gianopoulos [:WG9s] 2010-10-23 15:56:14 PDT
I thought it was about time I weighed in with my opinion on this.  First I want to provide a bit of background as to how we arrived at where we are.

The original idea was to make this a user placeable toolbar item with the default position being at the left hand side of the tabbar with the idea being that, although placing it on the tabbar might not make sense for a menu, with the tabs-on-top layout that would put the menu button at the upper left of the window, which was the most logical location.  If the user felt this should be someplace else, it could easily be moved.

It was then found that at several places in the code, #ifdef MENUBAR_CAN_AUTOHIDE was used to ensure that document.getElementById of appmenu items would not be null.  The problem is that if the button we removed form the toolbars and placed in the toolbar palette bu the user, that is exactly what would happen.  So either null checks or try/catch needed to be added to avoid exceptions.  As an example of this problem, private browsing mode failed to correctly initialize until I fixed the code there when I first implemented this.

So, a decision was made to make it non user (re)movable.

So, that is the current state of the patch here.

If we now want to move this to the toolbar, I have no idea where it would make sense to put it on the toolbar as a non-movable item.  The far left seems to NOT be a good idea at all based on all the complaints we have in the past when we accidentally had extra padding or something to the left of the back button that prevented users form being able to do a back by pinning the mouse on the screen left when the screen is is maximized mode.  I suspect that if we place a new item to the left of the back button a riot might ensue.

Therefore, at this point I think the choices are:

1.  Land the patch as is.

2.  Go back to making the item (re)movable and come up with a logical default location (which might be just leave it in the palette).

3.  Do nothing and I will try to implement this as an extension after the content and layout of the menu is finalized.
Comment 270 Dão Gottwald [:dao] 2010-10-23 23:13:47 PDT
The maintenance cost is pretty low since the interesting code is shared across platforms. To me being able to hack the menu on Linux would in fact be a help.

An add-on would need to start from scratch and I wouldn't encourage Linux distributors to develop and ship their own.
Comment 271 Mike Beltzner [:beltzner, not reading bugmail] 2010-10-23 23:33:35 PDT
(In reply to comment #268)
> Trying to balance cross platform UI consistency while also meshing with the
> interactive design with the surrounding platform is in many cases (like this
> one) simply not possible.  There are however a lot of reasons why users would

I guess the sticking point for me is: why Linux but not OS X? I can understand why we're making it the default in Windows 7 and Vista, and understand how we get from there to making it available (but not default) in XP.

Is it the case that we're saying since OS X has a global menu bar, we're not interested in implementing the Firefox button to save on UI space since it's not a concern on that platform? And thus on Linux, which may or may not have that global element (depending on the distro and what you believe will or will not happen with future versions of Ubuntu) we are providing it, but not turning it on by default?

> Placing in the tabstrip isn't ideal, but without the ability to draw in the
> title bar this was the only way for us to expose an app level control at the
> right level of the interface.  We could however just ignore that for the time
> being and put it in the current tab's navigation toolbar just to free up some
> tabstrip space.  I'm mostly indifferent with a slight preference towards
> placing it at the right level at the cost of taking some tabstrip space.

Placing it in the navigation toolbar doesn't make sense to me at all with tabs on top, no. If we do place it in the tabstrip, it must appear as it does on other platforms ("Firefox" with a down arrow), though, not as a single icon. That's going to cut into tabstrip space, which will be a tradeoff that I guess we let users decide.

Looks like we're moving forward, here, thanks for the clarification.
Comment 272 Bill Gianopoulos [:WG9s] 2010-10-24 03:46:48 PDT
(In reply to comment #271)
> I guess the sticking point for me is: why Linux but not OS X? I can understand
> why we're making it the default in Windows 7 and Vista, and understand how we
> get from there to making it available (but not default) in XP.

My early patches from when this was user placeable, added it for all platforms except Windows.  Under Linux it was placed by default on the tabbar, for other OS's it was just left unplaced in the toolbar palette.  During patch review, it was decided to keep this bug Linux only and allow the UI people for other platforms make their own decision on what to do in separate bugs.
Comment 273 Dão Gottwald [:dao] 2010-10-24 05:45:19 PDT
This pretty much sums up my point of view:

> Is it the case that we're saying since OS X has a global menu bar, we're not
> interested in implementing the Firefox button to save on UI space since it's
> not a concern on that platform? And thus on Linux, which may or may not have
> that global element (depending on the distro and what you believe will or will
> not happen with future versions of Ubuntu) we are providing it, but not turning
> it on by default?

A global menu bar on Linux would likely need our explicit support like on OS X. Once we provide that and depending on the desktop environment, we'd remove the "menu bar" toggle that this bug's patch adds.
Comment 274 Nicolas Mandil (:NicolasWeb) 2010-10-24 07:13:34 PDT
If I well  remember, we want too the firefox button to be implemented in Linux because it could drive to addons fracture between Linux and Windows (as Linux won't benefit of windows addons ; a few addons developer will develop specific overlays for Linux ; Linux don't have a global menu like OSX).
Comment 275 Bill Gianopoulos [:WG9s] 2010-10-24 17:03:40 PDT
Created attachment 485642 [details] [diff] [review]
patch v10-use text label with brandShortName instead of icon

Notes on this patch:

1.  I made no attempt to theme the Firefox button.  I figured any attempt on my part was unlikely to be what was decided on and only delay getting the patch landed so that the ui team could theme this as desired.

2.  I segregated the css code into code required to make things work, which I put into browser/base/content/browser.css from css code to define the look and feel which I put into browser/themes/gnomestripe/browser/browser.css.  This results in the Firefox button working correctly with most third party themes in a way that it did not with the previous patches I attached here.

3.  I should note, before I am asked, that it was intentional to remove the toolbarbutton-1 class from the button.  I looked through the source with mxr and found that all of the extra styling that the toolbarbutton-1 class was adding was things that I was overriding in css code in the gnomestripe theme, so that seemed counterproductive.
Comment 276 Bill Gianopoulos [:WG9s] 2010-10-24 17:13:02 PDT
Created attachment 485643 [details]
screenshot showing new button on tabbar - patch v10
Comment 277 Bill Gianopoulos [:WG9s] 2010-10-24 18:13:11 PDT
Created attachment 485647 [details]
screenshot of themed button

Here is the userChrome.css code I used to do this.  If this is considered better than the non-themed patch, I can add this.

#appmenu-toolbar-button {
  -moz-appearance: none !important;
  color: white !important;
  font-weight: bold !important;
  background: -moz-linear-gradient(rgb(247,182,82), rgb(215,98,10) 95%);
}
Comment 278 Bill Gianopoulos [:WG9s] 2010-10-24 18:52:48 PDT
Created attachment 485648 [details] [diff] [review]
patch v11-including my attempt to theme the button

I decided to just submit the patch for review including my themeing attempt.  Since I am asking for ui review also, I guess this can be discussed via the ui review process.
Comment 279 Dão Gottwald [:dao] 2010-10-25 00:43:49 PDT
Attachment 485643 [details] looks way better than attachment 485647 [details] to me.
Comment 280 Bill Gianopoulos [:WG9s] 2010-10-25 10:42:41 PDT
(In reply to comment #279)
> Attachment 485643 [details] looks way better than attachment 485647 [details] to me.

I am actually not particularly a fan of either one.  The first one seems to not have enough prominence to differentiate it from other items on the tab bar.  Since Beltzner is concerned that this does not logically belong on the tabs bar, it would be nice to have more visual difference than my first option provides.

I also suspect he would prefer it to look more like the windows implementation than this does.

The second choice I provided, I think is too far int he other direction.  I just took the windows styling and then copied it and added bolding the application name because it did not look as well with normal weight white on the background as it does under windows.

This one I think is a bit too distinctive, the orange is way too prominent and it is therefore distracting.  I am also not sure that the white text looks right given that the drop-marker is still black.

I think maybe going back to the first choice and figuring out some way to add maybe an slight Firefox orange glow tot he button is maybe a good compromise.
Comment 281 Alex Faaborg [:faaborg] (Firefox UX) 2010-10-25 14:03:59 PDT
>I guess the sticking point for me is: why Linux but not OS X? 

The only reason is that Bill is working on this patch for Linux, while we haven't had anyone pick up the work on OS X yet.
Comment 282 Alex Faaborg [:faaborg] (Firefox UX) 2010-10-25 14:07:02 PDT
>Attachment 485643 [details] looks way better than attachment 485647 [details] to me.

I agree, let's land this without any stronger visually styling for the time being.  We can try to follow up to give it more prominence later, but the range of OS themes on Linux will also somewhat limit what we are able to do.
Comment 283 Alex Faaborg [:faaborg] (Firefox UX) 2010-10-25 14:08:57 PDT
Comment on attachment 485648 [details] [diff] [review]
patch v11-including my attempt to theme the button

Let's roll back to the un-themed button in v10.  Without any significant interface changes you can feel free to carry the previous ui-reviews forward so we can get this landed once it passes code review.
Comment 284 Bill Gianopoulos [:WG9s] 2010-10-25 14:39:22 PDT
Comment on attachment 485642 [details] [diff] [review]
patch v10-use text label with brandShortName instead of icon

I was going to suggest this myself.  I realized today that the v10 patch was better because it was easier to play with approaches to add some orange via userChrome.css, if that is what we want, to something that does not have extraneous themeing.
Comment 285 Dão Gottwald [:dao] 2010-10-26 01:00:09 PDT
Comment on attachment 485642 [details] [diff] [review]
patch v10-use text label with brandShortName instead of icon

The label increases the tab bar height; it shouldn't do that.

You need to update aboutPrivateBrowsing.xhtml, which uses appMenuButton.label.

>+#appmenu-toolbar-button > .toolbarbutton-text {
>+  display: inherit;
>+}

s/inherit/-moz-box/
Comment 286 Bill Gianopoulos [:WG9s] 2010-10-26 16:24:23 PDT
(In reply to comment #285)
> Comment on attachment 485642 [details] [diff] [review]
> patch v10-use text label with brandShortName instead of icon
> 
> The label increases the tab bar height; it shouldn't do that.

Fixed.  I should have caught that.  I put the change for this in the gnomestripe theme as I think the height of the tabbar is a theme thing even though I had put the change to showing the label in browser/base/content as I determined that change to really be theme independent, at least for the default.

 
> You need to update aboutPrivateBrowsing.xhtml, which uses appMenuButton.label.

Good catch!
 
> >+#appmenu-toolbar-button > .toolbarbutton-text {
> >+  display: inherit;
> >+}
> 
> s/inherit/-moz-box/

Fixed.
Comment 287 Bill Gianopoulos [:WG9s] 2010-10-26 16:26:59 PDT
Created attachment 486218 [details] [diff] [review]
patch v12-fix button height and other review issues

Carrying faaborg's ui-review+ forward.
Comment 288 Bill Gianopoulos [:WG9s] 2010-10-26 17:00:05 PDT
Comment on attachment 486218 [details] [diff] [review]
patch v12-fix button height and other review issues

Oops.  Accidentally built a patch with all my pending patches instead of only those for this bug.  New patch forthcoming.
Comment 289 Bill Gianopoulos [:WG9s] 2010-10-27 03:05:38 PDT
Created attachment 486313 [details] [diff] [review]
patch v12a-fix button height and other review issues

Correct patch.  Once again carrying faaborg's ui-review+ forward.
Comment 290 Dão Gottwald [:dao] 2010-10-29 03:39:45 PDT
Comment on attachment 486313 [details] [diff] [review]
patch v12a-fix button height and other review issues

>--- a/browser/components/privatebrowsing/content/aboutPrivateBrowsing.xhtml
>+++ b/browser/components/privatebrowsing/content/aboutPrivateBrowsing.xhtml
>@@ -40,22 +40,21 @@
>   <!ENTITY % htmlDTD PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "DTD/xhtml1-strict.dtd">
>   %htmlDTD;
>   <!ENTITY % netErrorDTD SYSTEM "chrome://global/locale/netError.dtd">
>   %netErrorDTD;
>   <!ENTITY % globalDTD SYSTEM "chrome://global/locale/global.dtd">
>   %globalDTD;
>   <!ENTITY % browserDTD SYSTEM "chrome://browser/locale/browser.dtd">
>   %browserDTD;
>-#ifdef XP_WIN
>   <!ENTITY basePBMenu.label   "<span class='appMenuButton'>&brandShortName;</span><span class='toolsMenu'>&toolsMenu.label;</span>">
>-#elifdef XP_MACOSX
>+#ifdef XP_MACOSX
>   <!ENTITY basePBMenu.label   "&toolsMenu.label;">
> #else
>-  <!ENTITY basePBMenu.label   "<span class='appMenuButton'>&appMenuButton.label;</span><span class='toolsMenu'>&toolsMenu.label;</span>">
>+  <!ENTITY basePBMenu.label   "<span class='appMenuButton'>&brandShortName;</span><span class='toolsMenu'>&toolsMenu.label;</span>">
> #endif

This is broken (defines basePBMenu.label twice).
Comment 291 Bill Gianopoulos [:WG9s] 2010-10-29 05:12:47 PDT
(In reply to comment #290)

> This is broken (defines basePBMenu.label twice).

Sigh ... Looks like I can't even cut and paste correctly.  Lucky version 13 coming up.
Comment 292 Bill Gianopoulos [:WG9s] 2010-10-29 06:37:19 PDT
Created attachment 486897 [details] [diff] [review]
patch v13-fix review issues

Once again, Carrying faaborg's ui-review+ forward.
Comment 293 Bill Gianopoulos [:WG9s] 2010-10-29 06:44:05 PDT
Created attachment 486899 [details] [diff] [review]
interdiff v12a to v13
Comment 294 Dave Garrett 2010-10-29 08:35:53 PDT
(In reply to comment #291)
> Sigh ... Looks like I can't even cut and paste correctly.

Do anything a thousand times and you'll get better at it, but will eventually not notice the 1001th time may be an error. ;)

Thanks for your hard work on this patch!
Comment 295 Bill Gianopoulos [:WG9s] 2010-10-30 14:59:05 PDT
(In reply to comment #251)
> >this was not made a a design decision that menus should not have
> >icon, it was in fact, a performance improvement suggestion that since menus
> >open faster without icons
> 
> There's a human performance aspect here as well, if you only display icons for
> the highly used items, they are easier for the user to spot (compared to using
> icons for all of the items), so users are more likely to be able to quickly
> find the target they are looking for.  For instance on windows the context menu
> of the recycle bin places an icon next to "empty recycle bin" but not on any of
> the other items in the menu.  This visual search optimization is why we only
> included icons for some of the items in the Firefox menu.

The patch for bug 604650 results in icons only being displayed for menuitems with the menuitem-iconic-tooltip class instead of for all menutimems
Comment 296 Dão Gottwald [:dao] 2010-11-01 08:19:02 PDT
I landed the part which moves the menu popup code to browser-appmenu.inc, as this eases dealing with the code regardless of this bug:

http://hg.mozilla.org/mozilla-central/rev/443276883c2d
Comment 297 Bill Gianopoulos [:WG9s] 2010-11-01 11:21:26 PDT
Should I attach a new patch minus the landed part carrying reviews forward?
Comment 298 Bill Gianopoulos [:WG9s] 2010-11-01 11:26:47 PDT
Now that I see bug 608717, I will put a new patch here omitting both the already landed part, and the part I end up moving to bug 608717, and make this bug dependent on that one.
Comment 299 Bill Gianopoulos [:WG9s] 2010-11-01 16:24:28 PDT
Created attachment 487459 [details] [diff] [review]
patch v14-remove code already landed and that split off with bug 608717

I will ask for review once this passes my tests.
Comment 300 Bill Gianopoulos [:WG9s] 2010-11-01 16:35:12 PDT
Created attachment 487462 [details] [diff] [review]
patch v14-remove code already landed and that split off with bug 608717

This should be closer.
Comment 301 Bill Gianopoulos [:WG9s] 2010-11-01 19:22:36 PDT
Created attachment 487502 [details] [diff] [review]
patch v15-remove code already landed and that split off with bug 608717

Once again, carrying faaborg's ui-review+ forward.
Comment 302 Bill Gianopoulos [:WG9s] 2010-11-01 20:19:13 PDT
Created attachment 487512 [details] [diff] [review]
patch v16-update to merge with latest bug 608717 patch
Comment 303 Dão Gottwald [:dao] 2010-11-02 03:39:56 PDT
Comment on attachment 487512 [details] [diff] [review]
patch v16-update to merge with latest bug 608717 patch

>--- a/browser/base/content/browser.css
>+++ b/browser/base/content/browser.css

>+#appmenu-toolbar-button > .toolbarbutton-text {
>+  display: -moz-box;
>+}
> #appmenu_offlineModeRecovery:not([checked=true]) {
>   display: none;
> }

Can you add the appropriate ifdefs here?

>--- a/toolkit/themes/gnomestripe/global/global.css
>+++ b/toolkit/themes/gnomestripe/global/global.css

>-toolbar[type="menubar"]:not(-moz-lwtheme):-moz-system-metric(menubar-drag) {
>+toolbar[type="menubar"]:not([autohide="true"]):not(-moz-lwtheme):-moz-system-metric(menubar-drag) {
>   -moz-binding: url("chrome://global/content/bindings/toolbar.xml#toolbar-drag");
> }

This will need to merge with bug 608522.
Comment 304 Bill Gianopoulos [:WG9s] 2010-11-02 06:05:33 PDT
(In reply to comment #303)
> Comment on attachment 487512 [details] [diff] [review]
> patch v16-update to merge with latest bug 608717 patch
> 
> >--- a/browser/base/content/browser.css
> >+++ b/browser/base/content/browser.css
> 
> >+#appmenu-toolbar-button > .toolbarbutton-text {
> >+  display: -moz-box;
> >+}
> > #appmenu_offlineModeRecovery:not([checked=true]) {
> >   display: none;
> > }
> 
> Can you add the appropriate ifdefs here?

Sure!

> 
> >--- a/toolkit/themes/gnomestripe/global/global.css
> >+++ b/toolkit/themes/gnomestripe/global/global.css
> 
> >-toolbar[type="menubar"]:not(-moz-lwtheme):-moz-system-metric(menubar-drag) {
> >+toolbar[type="menubar"]:not([autohide="true"]):not(-moz-lwtheme):-moz-system-metric(menubar-drag) {
> >   -moz-binding: url("chrome://global/content/bindings/toolbar.xml#toolbar-drag");
> > }
> 
> This will need to merge with bug 608522.

OK.  I'll post a new patch tonight.
Comment 305 Bill Gianopoulos [:WG9s] 2010-11-02 18:26:33 PDT
Created attachment 487791 [details] [diff] [review]
patch v17;ui-review=faaborg
Comment 306 Dão Gottwald [:dao] 2010-11-03 00:44:18 PDT
Comment on attachment 487791 [details] [diff] [review]
patch v17;ui-review=faaborg

>--- a/browser/base/content/browser.css
>+++ b/browser/base/content/browser.css
>@@ -140,16 +140,23 @@ toolbar[mode="icons"] > #reload-button[d
> }
> 
> .menuitem-iconic-tooltip,
> .menuitem-tooltip[type="checkbox"],
> .menuitem-tooltip[type="radio"] {
>   -moz-binding: url("chrome://browser/content/urlbarBindings.xml#menuitem-iconic-tooltip");
> }
> 
>+%ifdef MENUBAR_CAN_AUTOHIDE
>+%ifndef CAN_DRAW_IN_TITLEBAR
>+#appmenu-toolbar-button > .toolbarbutton-text {
>+  display: -moz-box;
>+}
>+%endif
>+%endif
> #appmenu_offlineModeRecovery:not([checked=true]) {
>   display: none;
> }

Extend MENUBAR_CAN_AUTOHIDE to include #appmenu_offlineModeRecovery, add a blank line before #appmenu_offlineModeRecovery.
Comment 307 Bill Gianopoulos [:WG9s] 2010-11-03 07:01:46 PDT
Created attachment 487892 [details] [diff] [review]
patch v18;r=dao,ui-review=faaborg

Thanks for the review Dão.

Carrying reviews forward.  Looking for approval to land this and associated patches as soon as the trunk opens for beta8 to maximize the bake time on nightlies before this goes out in a beta.
Comment 308 Yann Brelière 2010-11-03 07:34:24 PDT
Isn't beta 7 the feature freeze? If so, it should land for beta 7?
Comment 309 Bill Gianopoulos [:WG9s] 2010-11-03 11:52:07 PDT
(In reply to comment #308)
> Isn't beta 7 the feature freeze? If so, it should land for beta 7?

Beta7 is done.  There will be no more bugs accepted for inclusion in beta 7 except for serious regressions.
Comment 310 Dave Garrett 2010-11-04 01:22:25 PDT
(In reply to comment #308)
> Isn't beta 7 the feature freeze? If so, it should land for beta 7?

A strings patch has already landed in this bug (a cheat to get around freezes) and the feature in question has been available on Windows for quite some time. "Feature freeze" means no new features, not no porting to other OSes. This would have to land for a beta, but not necessarily 7, hopefully 8 at this pace.
Comment 311 Yann Brelière 2010-11-04 02:36:16 PDT
Ok :) Thanks for the clarification.
Comment 312 Dão Gottwald [:dao] 2010-11-07 06:15:15 PST
appMenuButton.label removal: http://hg.mozilla.org/mozilla-central/rev/28b1e8b3f538
Comment 313 Dão Gottwald [:dao] 2010-11-12 02:01:04 PST
Created attachment 490053 [details] [diff] [review]
patch, updated to tip
Comment 314 antistress 2010-11-14 11:39:04 PST
using the latest build on Bill's server i have noticed that Firefox still alows the user to untick the preference that always shows the tab bar.

Then, having the menu button sitting in the tab bar instead of the toolbar let the user without any menu (menu bar nor menu button) if the preference is unticked and the user only has a tab.

Maybe the menu button should sit it the toolbar ?
Comment 315 Dave Garrett 2010-11-14 12:01:57 PST
(In reply to comment #314)
> using the latest build on Bill's server i have noticed that Firefox still alows
> the user to untick the preference that always shows the tab bar.

That should be going away at some point in bug 594614.
Comment 316 Justin Dolske [:Dolske] 2010-11-19 17:30:55 PST
Has the visual appearance / interaction of this patch changed since faaborg ui-r+'d the screenshot of patch v9 (attachment 484611 [details])? I see the latest patch is v.19. I'm hesitant to a+ this until that's clear.
Comment 317 Bill Gianopoulos [:WG9s] 2010-11-20 03:42:56 PST
(In reply to comment #316)
> Has the visual appearance / interaction of this patch changed since faaborg
> ui-r+'d the screenshot of patch v9 (attachment 484611 [details])? I see the latest patch
> is v.19. I'm hesitant to a+ this until that's clear.

The only major visual appearance / interaction since v9 was the change form an icon for the firefox button, to a text button which was done in the v10 patch.  faaborg approved that in comment 289.

I am attaching a screenshot similar to that shown for the v9 patch to show that is the only change.

If you want, I can ask faaborg for one last ui-review.
Comment 318 Bill Gianopoulos [:WG9s] 2010-11-20 03:44:06 PST
Created attachment 492048 [details]
screenshot with menu open - patch v19
Comment 319 Bill Gianopoulos [:WG9s] 2010-11-20 12:22:59 PST
(In reply to comment #317)
> (In reply to comment #316)
> > Has the visual appearance / interaction of this patch changed since faaborg
> > ui-r+'d the screenshot of patch v9 (attachment 484611 [details] [details])? I see the latest patch
> > is v.19. I'm hesitant to a+ this until that's clear.
> 
> The only major visual appearance / interaction since v9 was the change form an
> icon for the firefox button, to a text button which was done in the v10 patch. 
> faaborg approved that in comment 289.
                           ^^^^^^^^^^^

I meant comment 283.
Comment 320 Reed Loden [:reed] (use needinfo?) 2010-11-20 16:42:27 PST
http://hg.mozilla.org/mozilla-central/rev/0601ba3e9395
Comment 321 :Ehsan Akhgari (busy, don't ask for review please) 2010-11-20 17:46:53 PST
The landing of this bug broke the mozmill suite on mozilla-central.  Because we do not have a clear policy on how to deal with these types of failures, and we don't run mozmill tests on the try server, I just filed bug 613802 for the failure, and hid the Linux mozmill boxes on mozilla-central, and let the patch remain on trunk.
Comment 322 :Ehsan Akhgari (busy, don't ask for review please) 2010-11-20 18:07:48 PST
This also caused a 2.92% Txul regression on Linux:

Regression :( Txul increase 2.92% on Linux Firefox
--------------------------------------------------
    Previous: avg 93.972 stddev 0.501 of 30 runs up to revision 8af7288622ea
    New     : avg 96.716 stddev 0.177 of 5 runs since revision 0601ba3e9395
    Change  : +2.744 (2.92% / z=5.475)
    Graph   : http://mzl.la/aHwNFc

Changeset range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8af7288622ea&tochange=0601ba3e9395
Comment 323 Bill Gianopoulos [:WG9s] 2010-11-20 19:00:43 PST
(In reply to comment #321)
> The landing of this bug broke the mozmill suite on mozilla-central.  Because we
> do not have a clear policy on how to deal with these types of failures, and we
> don't run mozmill tests on the try server, I just filed bug 613802 for the
> failure, and hid the Linux mozmill boxes on mozilla-central, and let the patch
> remain on trunk.

This is surprising.  It will be interesting to find out what happened here.
Comment 324 Bill Gianopoulos [:WG9s] 2010-11-20 19:02:47 PST
(In reply to comment #322)
> This also caused a 2.92% Txul regression on Linux:
> 
> Regression :( Txul increase 2.92% on Linux Firefox
> --------------------------------------------------
>     Previous: avg 93.972 stddev 0.501 of 30 runs up to revision 8af7288622ea
>     New     : avg 96.716 stddev 0.177 of 5 runs since revision 0601ba3e9395
>     Change  : +2.744 (2.92% / z=5.475)
>     Graph   : http://mzl.la/aHwNFc
> 
> Changeset range:
> http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8af7288622ea&tochange=0601ba3e9395

This is disturbing.  Especially since, by default, the old menu is displayed.  Was there a similar Windows Txul regression when the new menu and menubar autohide landed under Windows?
Comment 325 Vykintas Vyšniauskas 2010-11-21 00:27:46 PST
The button is added to the very right of the tab bar, and it doesn't seem to be movable via the customize dialog. How can I move it to the very left?
Comment 326 Dão Gottwald [:dao] 2010-11-21 00:31:26 PST
(In reply to comment #324)
> Was there a similar Windows Txul regression when the new menu and menubar
> autohide landed under Windows?

I don't think so.

(In reply to comment #325)
> The button is added to the very right of the tab bar, and it doesn't seem to be
> movable via the customize dialog.

This happens if you customized your toolbars in a previous 4.0 beta / nightly.

> How can I move it to the very left?

"Restore default set" should do it.
Comment 327 Vykintas Vyšniauskas 2010-11-21 00:36:06 PST
(In reply to comment #326)
> "Restore default set" should do it.

Worked, thanks.
Comment 328 Reed Loden [:reed] (use needinfo?) 2010-11-21 01:21:26 PST
(In reply to comment #326)
> (In reply to comment #324)
> > Was there a similar Windows Txul regression when the new menu and menubar
> > autohide landed under Windows?
> 
> I don't think so.

Bug 575093 says otherwise.
Comment 329 Dão Gottwald [:dao] 2010-11-21 01:23:20 PST
Bug 575093 says "25% TXul regression from hiding the menu bar". We're not hiding the menu bar on Linux -- not by default.
Comment 330 Dão Gottwald [:dao] 2010-11-21 01:26:28 PST
I pushed this to the try server to see if it helps txul, hoping "-t chrome" makes it run txul: http://hg.mozilla.org/try/rev/7413f7c67643
Comment 331 Jim Mathies [:jimm] 2010-11-21 08:21:12 PST
(In reply to comment #324)
> This is disturbing.  Especially since, by default, the old menu is displayed. 
> Was there a similar Windows Txul regression when the new menu and menubar
> autohide landed under Windows?

We had a couple regressions on windows, one from some unnecessary xpcom overhead in the widget patches, and another from an extra reflow that was triggered. The total regression was around 5%, 3% xpcom, 2% reflow.
Comment 332 Michael Monreal [:monreal] 2010-11-21 12:57:02 PST
Maybe someone wants to have a look at bug #613880... this way submenus are
positioned looks bad on this new menu.
Comment 333 Dão Gottwald [:dao] 2010-11-22 02:04:03 PST
(In reply to comment #330)
> I pushed this to the try server to see if it helps txul, hoping "-t chrome"
> makes it run txul: http://hg.mozilla.org/try/rev/7413f7c67643

Didn't help, as far as I could interpret the numbers.
Comment 334 Michael Monreal [:monreal] 2010-11-25 03:50:38 PST
(In reply to comment #325)
> The button is added to the very right of the tab bar, and it doesn't seem to be
> movable via the customize dialog. How can I move it to the very left?

Funny enough, I like the button so much better on the right side. Is there a hack to always have it there? I will have to press reset to default sooner or later...
Comment 335 Bill Gianopoulos [:WG9s] 2010-11-25 05:07:31 PST
(In reply to comment #334)
> Funny enough, I like the button so much better on the right side. Is there a
> hack to always have it there? I will have to press reset to default sooner or
> later...

The easiest way to move it would be to make some customization on the Tabs toolbar then exit the browser and edit the localstore.rdf file.  In this file, find the TabsToolbar RDF entry and edit the currentset by inserting/moving "appmenu-toolbar-button" at the desired location.
Comment 336 antistress 2010-12-10 02:55:37 PST
For the record, note that the decision to keep menu bar by default on Linux (i don't criticize that decision) makes bug 606744#c6 very sensible on Linux  : panorama button moves up and down depending if Panorama mode is ON or OFF with menu bar displayed.
Comment 337 Amit Prakash Ambasta 2011-03-10 23:31:51 PST
gtk3 has been out for a while and it would be desirable if ff4 or a closer update to ff4 could handle drawing the ff button on the title bar
Comment 338 George Carstoiu 2011-04-04 23:46:55 PDT
Verified the presence of the Firefox button with Mozilla/5.0 (X11; Linux i686; rv:2.2a1pre) Gecko/20110404 Firefox/4.2a1pre
Comment 339 Francisco 2011-05-19 06:23:50 PDT
Though this bug has already been marked as fixed, I have something to add: the menu button on KDE.

Don't know if it has been addressed anywhere else but I haven't seen this here except for the comment of Dave Garrett. 

It *is* already possible to draw the button on the windows decoration. It seems to be that it's planned for KDE 4.8 but it's already here:

http://kde-look.org/content/show.php?content=141254&forumpage=0

And it looks like this:
http://img88.imageshack.us/i/menubtn3.jpg/

The only problem is that when it's drop down, it has the classic manu inside:
"File>
 Edit>
 etc>.."

(I can't take a screenshot right now... it doesn't let me when I have the menu pressed)

But, if there's a addon or something, special for KDE, that show the classic menu as the "new menu", so it will draw the new menu at the end.

Just an option.
Comment 340 Dave Garrett 2011-05-19 10:56:49 PDT
(In reply to comment #339)
> Though this bug has already been marked as fixed, I have something to add:
> the menu button on KDE.
> 
> Don't know if it has been addressed anywhere else but I haven't seen this
> here except for the comment of Dave Garrett.

Did you just mean mention of KDE or something else specific?

> (I can't take a screenshot right now... it doesn't let me when I have the
> menu pressed)

I listed the steps to do that under KDE up in comment 132.

> But, if there's a addon or something, special for KDE, that show the classic
> menu as the "new menu", so it will draw the new menu at the end.

Nobody here is likely to write an addon for this, and while I do personally prefer KDE, Firefox is a GTK application (though not GNOME; that distinction needs to be made). The KDE stuffs always gets the back seat, unfortunately, though I can't really disagree with that being the case. When drawing the Firefox button in the title bar is done on Linux I would expect it to be done in a cross-DE way via GTK features (which was discussed above somewhere).

In any case, this bug is long and winding and done. Whomever starts work on this will do so in a new bug.
Comment 341 ra.ravi.rav 2011-05-19 11:16:35 PDT
@Francisco I like the new KDE menu and I guess the classic menu this way is a welcome change. But yeah I agree with Dave, KDE gets a backseat most of the times.
Comment 342 Francisco 2011-05-19 11:23:51 PDT
> Did you just mean mention of KDE or something else specific?

Sorry. Yes, I was referring KDE. 


> I listed the steps to do that under KDE up in comment 132.

Ok. Here's the screenshot
http://www.imagengratis.org/images/ffmenu.png

(the most embarrassing part is that I had already read comment 132...)
 
> Nobody here is likely to write an addon for this, and while I do personally
> prefer KDE, Firefox is a GTK application (though not GNOME; that distinction
> needs to be made). The KDE stuffs always gets the back seat, unfortunately,
> though I can't really disagree with that being the case. When drawing the
> Firefox button in the title bar is done on Linux I would expect it to be
> done in a cross-DE way via GTK features (which was discussed above
> somewhere).
> 
> In any case, this bug is long and winding and done. Whomever starts work on
> this will do so in a new bug.

Well, after reading what I wrote about a new "addon" I realized it was quite silly from me. I was referring that maybe an addon would be enough for getting it work.

I understand your point. It's just I followed this thread when it first appeared and the "GTK2 can't draw buttons in the title bar" last year or so. 

When I knew that (at least in KDE) it was already possible to draw the button in the title bar, I remembered this thread and tried to add something. And it's a pitty, because it's possible to put tabs con title bar also. But well, I understand that FF is s GTK app. I just tried to add more information.

Regards

Note You need to log in before you can comment on or make changes to this bug.