97549 - mac: Text Size (100%) menu label doesn't update

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[sairuh (rarely reading bugmail)]

spun off of bug 39117. i've see this on Mac OS X [2001.08.29.05-comm] and on Mac
OS 9.1 [emul over X, 2001.08.29.08-comm].

after changing the text size --either by hitting cmd+plus/minus or selecting
from the View > Text Size submenu-- the label for the text size remains as Text
Size (100%).

*however* the actual submenu itself properly shows the selected text size with a
checkmark.

pls reassign as needed, thx!

Comment 1

[sairuh (rarely reading bugmail)]

mac only.

Comment 2

[John Morrison]

I just happened to be doing a build on Mac, for fastload reasons, and I 
noticed this message fly by when I was starting up the browser, each time. 

************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'[JavaScript Error: "this.bundle has no properties" {file: 
"chrome://communicator/content/viewZoomOverlay.js" line: 49}]' when
calling method: [nsIDOMEventListener::handleEvent]"  nsresult: "0x80570021 
(NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "<unknown>"
data: yes]
************************************************************

Where the code in question is:

 45 function ZoomManager() {
 46   this.bundle = document.getElementById("bundle_viewZoom");
 47 
 48   // factorAnchor starts on factorOther
 49   this.factorOther = parseInt(this.bundle.getString("valueOther"));
 50   this.factorAnchor = this.factorOther;
 51 }

and that means you got no ZoomManager obj on the Mac (for whatever reason -- 
overlay badness, build errors, phase of moon?)

Comment 3

[Peter Van der Beken [:peterv]]

Hmm, weird. Debugged this a bit. I had to change one this.value to
this.setAttribute("value") (anyone know why?), and i can see the label attribute
for the menu item getting set, but the menu doesn't change. I'll attach a patch
that shows what happens.

Comment 4

[Peter Van der Beken [:peterv]]

Attachment: .value -> .getAttribute("value") and adding dumps

Comment 5

[jag (Peter Annema)]

I don't quite see why you had to change the .value to .getAttribute('value'), if
that were broken you wouldn't be able to switch text zoom at all. Did that work
for you before your change?

Comment 6

[jag (Peter Annema)]

-> pinkerton

Mac rulez!

Comment 7

[Mike Pinkerton (not reading bugmail)]

r=saari. here's the patch

Index: mozilla/widget/src/mac/nsMenu.cpp
===================================================================
RCS file: /m/pub/mozilla/widget/src/mac/nsMenu.cpp,v
retrieving revision 1.102
diff -u -2 -r1.102 nsMenu.cpp
--- nsMenu.cpp	2001/08/17 09:44:53	1.102
+++ nsMenu.cpp	2001/08/30 22:31:36
@@ -1485,6 +1485,12 @@
     mMenuContent->GetAttr(kNameSpaceID_None, nsWidgetAtoms::label, mLabel);
 
-    if((mMacMenuID <= 5) && (mMacMenuID >= 2)) 
+    // if we're a submenu, we don't have to go through all the gymnastics below
+    // to remove ourselves from the menubar and re-add the menu. We just 
invalidate
+    // our parent to change the text of the item.
+    if((mMacMenuID <= 5) && (mMacMenuID >= 2)) {
+      nsCOMPtr<nsIMenuListener> listener = do_QueryInterface(mParent);
+      listener->SetRebuild(PR_TRUE);
       return NS_OK;
+    }
     
     ::DeleteMenu(mMacMenuID);

Comment 8

[Mike Pinkerton (not reading bugmail)]

needs sr. smfr?

Comment 9

[Simon Fraser [no longer active]]

Comment on attachment 47653 [details] [diff] [review]
.value -> .getAttribute("value") and adding dumps

sr=sfraser

Comment 10

[Simon Fraser [no longer active]]

I was sr'ing the patch in pinkerton's comment, and not the attachment. Really.

Comment 11

[Asa Dotzler [:asa]]

Comment on attachment 47653 [details] [diff] [review]
.value -> .getAttribute("value") and adding dumps

a=asa for checkin to 0.9.4 branch

Comment 12

[Mike Pinkerton (not reading bugmail)]

landed on trunk and branch.

Comment 13

[John Morrison]

verified 10/1 branch mac build.

Comment 14

[Rob Griffiths]

This comment doesn't belong with this bug, but I cannot find the bug for 'text
zoom not working on Mac'.  I know it's here somewhere...

Text zoom was working perfectly starting with (I think) 0.9.5 ... I just grabbed
last night's build (20011111506) and it now fails.  The sub-menu shows the sizes
being changed, but my onscreen text (on the page itself, I'm not talking about
the "100%" label in the menu) doesn't change on bit.  It worked fine in the 11/9
build...

G4/350 running 10.1.1.

-rob.