63042 - `Print' toolbar button shouldn't have a menu

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Matthew T (active 1999-2002)]

Build: 2000121514, Mac OS 9.0

The `Print' button in the toolbar has a menu with one (1) item in it,
`Print ...'. The menu serves no purpose and should not exist.

Comment 1

[HJ]

And with more than one printer??

Comment 2

[Matthew T (active 1999-2002)]

What and with more than one printer what?

Comment 3

[Marcia Knous [:marcia]]

Netscape nav triage team: as per Alec Flett's pre-triage recommendation, this 
bug is nsbeta1-.

Comment 4

[[e-mail] (gone - use [e-mail] instead)]

I take it this is a result of the comm build stripping of the Print Plus submenu?

Comment 5

[timeless]

we're supposed to have print setup, page setup and print preview. Don't remove 
the button.

mpt: stop filing bugs. it's ok for you to create specs and it's ok for you to 
advise about implementation and it's ok for you to tell us when we aren't 
meeting specs.  It is not ok for you to use bugzilla in this fassion.

recommend resolve wontfix/invalid.

Comment 6

[Matthew T (active 1999-2002)]

No previous version of Netscape, and no version of Internet Explorer, has ever 
had toolbar buttons for Print Setup, or Page Setup, or Print Preview -- for the 
very good reason that none of them are used nearly often enough to deserve to 
be on the toolbar.

> Don't remove the button.

No, I didn't say the button should be removed, just the menu.

> mpt: stop filing bugs.

How amusing.

Comment 7

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

timeless: quit with the crack already. 

Comment 8

[Zach Lipton [:zach]]

This should be fixed. Why is this mac only? I'll attach a patch in a sec, this
would be good and low risk enough to make 0.8.1 if drivers say ok. 

Comment 9

[Zach Lipton [:zach]]

This is a first-round patch: 

Index: mozilla/xpfe/browser/resources/content/navigator.xul
=========================================================
==========
RCS file: /cvsroot/mozilla/xpfe/browser/resources/content/navigator.xul,v
retrieving revision 1.283
diff -u -2 -r1.283 navigator.xul
--- mozilla/xpfe/browser/resources/content/navigator.xul	2001/03/14 
03:05:08	1.283
+++ mozilla/xpfe/browser/resources/content/navigator.xul	2001/03/18 
00:08:29
@@ -180,7 +180,4 @@
       <menubutton class="menubutton-dual toolbar top" id="print-button" 
persist="hidden" value="&printButton.label;" 
                   crop="right" buttonaction="BrowserPrint()" tooltip="aTooltip" 
tooltiptext="&printButton.tooltip;">  
-        <menupopup id="printMenu" popupalign="topright" popupanchor=
"bottomright">
-          <menuitem value="&printCmd.label;" accesskey="&
printCmd.accesskey;" oncommand="BrowserPrint()" default="true"/>
-        </menupopup>
       </menubutton>
 

The problem is that it still keeps the little triangle there as if the menu 
were there. Looking for a way arround this.

Comment 10

[Zach Lipton [:zach]]

I am so dumb. I missed the large MENU that's part of the tag. Try this:

Index: mozilla/xpfe/browser/resources/content/navigator.xul
=========================================================
==========
RCS file: /cvsroot/mozilla/xpfe/browser/resources/content/navigator.xul,v
retrieving revision 1.283
diff -u -2 -r1.283 navigator.xul
--- mozilla/xpfe/browser/resources/content/navigator.xul	2001/03/14 
03:05:08	1.283
+++ mozilla/xpfe/browser/resources/content/navigator.xul	2001/03/18 
00:14:21
@@ -178,10 +178,7 @@
       </hbox>
 
-      <menubutton class="menubutton-dual toolbar top" id="print-button" 
persist="hidden" value="&printButton.label;" 
+      <button class="menubutton-dual toolbar top" id="print-button" 
persist="hidden" value="&printButton.label;" 
                   crop="right" buttonaction="BrowserPrint()" tooltip="aTooltip" 
tooltiptext="&printButton.tooltip;">  
-        <menupopup id="printMenu" popupalign="topright" popupanchor=
"bottomright">
-          <menuitem value="&printCmd.label;" accesskey="&
printCmd.accesskey;" oncommand="BrowserPrint()" default="true"/>
-        </menupopup>
-      </menubutton>
+      </button>
 
       <hbox id="throbber-box" autostretch="never">

Comment 11

[Zach Lipton [:zach]]

Forgot to hack the theme to not use menubutton rules for print any more. 

Index: mozilla/themes/modern/navigator/navigator.css
=========================================================
==========
RCS file: /cvsroot/mozilla/themes/modern/navigator/navigator.css,v
retrieving revision 1.38
diff -u -2 -r1.38 navigator.css
--- mozilla/themes/modern/navigator/navigator.css	2001/03/16 02:50:33	
1.38
+++ mozilla/themes/modern/navigator/navigator.css	2001/03/18 00:46:45
@@ -107,28 +107,23 @@
 }
 
+
 #print-button {
-  -moz-binding: url("chrome://communicator/skin/
widgetBindings.xml#menubutton-dual-1");
-  margin: 6px 6px 0px 6px;
   list-style-image: url("chrome://communicator/skin/btn1/print.gif");
 }
 
 #print-button[disabled="true"],
-#print-button[disabled="true"] > .menubutton-dual-stack > .menubutton-
dual-button:hover,
-#print-button[disabled="true"] > .menubutton-dual-stack > .menubutton-
dual-button:hover:active {
+#print-button[disabled="true"]:hover,
+#print-button[disabled="true"]:hover:active {
   list-style-image: url("chrome://communicator/skin/btn1/print-dis.gif");
 }
 
-#print-button > .menubutton-dual-stack > .menubutton-dual-button:hover 
{
-  list-style-image: url("chrome://communicator/skin/btn1/print-hov.gif");
+#print-button:hover {
+  list-style-image: url("chrome://navigator/skin/btn1/print-hov.gif");
 }
 
-#print-button > .menubutton-dual-stack > .menubutton-dual-
button:hover:active {
-  list-style-image: url("chrome://communicator/skin/btn1/print-act.gif");
+#print-button:hover:active {
+  list-style-image: url("chrome://navigator/skin/btn1/print-act.gif");
 }
 
-#print-button > .menubutton-dual-stack > .menubutton-dual-dropmarker-
box {
-  margin-left: 19px;
-  margin-top: 22px;
-}
 
 /* Hides text below the above buttons */

Comment 12

[Zach Lipton [:zach]]

Attachment: I suck and I pasted that in by mistake. Here is a full patch to fix.

Comment 13

[Zach Lipton [:zach]]

Attachment: Forgot to update classic. Also, I can just use <menubutton/> instead of </menubutton>

Comment 14

[Zach Lipton [:zach]]

Hold off on this for now, it really breaks the skins. Trying to find out why?

Comment 15

[Zach Lipton [:zach]]

Attachment: Working patch. Note that the little triangle is still there. How do I get rid of it?

Comment 16

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

huzzah. a=ben

Comment 17

[Zach Lipton [:zach]]

Attachment: Finished and fully working patch!

Comment 18

[Mike Shaver (:shaver emeritus)]

Works great for me, and looks like solid work.

sr=shaver.

Comment 19

[Zach Lipton [:zach]]

a=drivers

Comment 20

[Zach Lipton [:zach]]

r=hwaara, let's land this baby!

Comment 21

[Zach Lipton [:zach]]

This patch is: 

r=hwaara

sr=shaver

a=ben

branchapproval=drivers

It is ready to checkin and is tested and proven to fix this problem.

Comment 22

[Hixie (not reading bugmail)]

Wow, all this excitement for the removal of an almost empty drop down menu?
I'd can't wait to see your enthusiasm when we get ready to land the new view
manager or something like that! ;-)

Comment 23

[Zach Lipton [:zach]]

Hixie: that is so funny I forgot to laugh. Note that the patch doesn't work,
don't check it in!

Comment 24

[Paul Chen]

Marking nsbeta1- bugs future, though other than the patch not working, sounds
like we're close to fixing this. ;-)

Comment 25

[Doron Rosenberg (IBM)]

Attachment: nw patch

Comment 26

[Doron Rosenberg (IBM)]

new patch attached.  Perhaps a macperson can test if the mac classic is ok...

any comments? there were some double entries as well that I removed.

Comment 27

[Blake Ross]

-> doron

Cc'ing alec to review his patch, if he can.

Comment 28

[Doron Rosenberg (IBM)]

This should be easy to get into 0.9

Comment 29

[Alec Flett]

Netscape needs this for Print Plus.. I don't know who did that work but we need
to find a way to have no menu in the mozilla build, and a menu in the commercial
build.

Comment 30

[Doron Rosenberg (IBM)]

i'll talk to the build people and see how to do this, if possible at all.

Comment 31

[Alec Flett]

it's totally possible with dynamic overlays, it has nothing to do with 'build
people' - the netscape commercial builds (including Netscape 6) already do this.

Comment 32

[Doron Rosenberg (IBM)]

my bad then, i thought it was just not checking into the commercial tree or
whatnot.  

Comment 33

[Doron Rosenberg (IBM)]

-> 1.0

Comment 34

[Alex Bishop]

Changing Platform/OS to all as this affects all systems.

Comment 35

[Alex Bishop]

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

Comment 36

[David Hallowell]

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

Comment 37

[Jesse Ruderman]

See also bug 111126, add "print preview" to this menu.

Comment 38

[:deanis74]

Now that bug 111126 has been checked in, doubling the number of items on the
menu, is this bug invalid?

Comment 39

[Ian Pottinger]

adding self to cc list

Comment 40

[shrirang khanzode]

am not complaining but...when u add urself to the cc list, u need not mention 
it explicitely. It might screw up someone's filters who does not want that extra 
mail notifying him that you added urself to the cc list :) cheers!

Comment 41

[:deanis74]

I emailed Ian last night and he responded that he was using the Change Multiple
Bugs function and was forced to add a comment before it would allow him to proceed.

Comment 42

[:deanis74]

This bug should be marked WONTFIX, now that there is a valid menu off of the
Print button, with Print and Print Preview.  See also bug 130066, which jives
with (the first part of) comment 5 and would add a third item to the menu.

Comment 43

[Matthew T (active 1999-2002)]

No, this bug is still valid. See comment 6. I have yet to see *any* program
(other than Mozilla) which is stupid enough to have a Print toolbar menu, no
matter how many items it has.

Comment 44

[Doron Rosenberg (IBM)]

we have 2 items now, goodbye!