TIP #533 (Extension of the menu post command) implementation was accepted by TCT vote. This allows fixing of bug [70e531918e]: geometry issues with menubuttons on macOS.

3 files changed, 271 insertions, 214 deletions

diff --git a/macosx/tkMacOSXDefault.h b/macosx/tkMacOSXDefault.h
index a0b6a79..dd6be69 100644
--- a/macosx/tkMacOSXDefault.h
+++ b/macosx/tkMacOSXDefault.h
@@ -334,7 +334,7 @@
  * Defaults for menubuttons:
  */
 
-#define DEF_MENUBUTTON_ANCHOR		"center"
+#define DEF_MENUBUTTON_ANCHOR		"w"
 #define DEF_MENUBUTTON_ACTIVE_BG_COLOR	ACTIVE_BG
 #define DEF_MENUBUTTON_ACTIVE_BG_MONO	BLACK
 #define DEF_MENUBUTTON_ACTIVE_FG_COLOR	ACTIVE_FG
@@ -342,7 +342,7 @@
 #define DEF_MENUBUTTON_BG_COLOR		NORMAL_BG
 #define DEF_MENUBUTTON_BG_MONO		WHITE
 #define DEF_MENUBUTTON_BITMAP		""
-#define DEF_MENUBUTTON_BORDER_WIDTH	"2"
+#define DEF_MENUBUTTON_BORDER_WIDTH	"0"
 #define DEF_MENUBUTTON_CURSOR		""
 #define DEF_MENUBUTTON_DIRECTION	"below"
 #define DEF_MENUBUTTON_DISABLED_FG_COLOR DISABLED
@@ -358,8 +358,8 @@
 #define DEF_MENUBUTTON_INDICATOR	"1"
 #define DEF_MENUBUTTON_JUSTIFY		"left"
 #define DEF_MENUBUTTON_MENU		""
-#define DEF_MENUBUTTON_PADX		"4"
-#define DEF_MENUBUTTON_PADY		"3"
+#define DEF_MENUBUTTON_PADX		"0"
+#define DEF_MENUBUTTON_PADY		"0"
 #define DEF_MENUBUTTON_RELIEF		"flat"
 #define DEF_MENUBUTTON_STATE		"normal"
 #define DEF_MENUBUTTON_TAKE_FOCUS	"0"
diff --git a/macosx/tkMacOSXMenu.c b/macosx/tkMacOSXMenu.c
index 8f3be9f..dfa3e53 100644
--- a/macosx/tkMacOSXMenu.c
+++ b/macosx/tkMacOSXMenu.c
@@ -755,10 +755,13 @@ TkpDestroyMenuEntry(
  *
  * TkpPostMenu --
  *
- *	Posts a menu on the screen
+ *	Posts a menu on the screen. If entry is < 0 then the menu is
+ *      drawn so its top left corner is located at the point with
+ *      screen coordinates (x, y).  Otherwise the top left corner of
+ *      the specified entry is located at that point.
  *
  * Results:
- *	None.
+ *	Returns a standard Tcl result.
  *
  * Side effects:
  *	The menu is posted and handled.
@@ -770,50 +773,52 @@ int
 TkpPostMenu(
     Tcl_Interp *interp,		/* The interpreter this menu lives in */
     TkMenu *menuPtr,		/* The menu we are posting */
-    int x,			/* The global x-coordinate of the top, left-
-				 * hand corner of where the menu is supposed
-				 * to be posted. */
-    int y)			/* The global y-coordinate */
+    int x, int y,		/* The screen coordinates where the top left
+				 * corner of the menu, or of the specified
+				 * entry, will be located. */
+    int index)
 {
+    int result;
+    Tk_Window root = Tk_MainWindow(interp);
 
-
-    /* Get the object that holds this Tk Window.*/
-    Tk_Window root;
-    root = Tk_MainWindow(interp);
     if (root == NULL) {
 	return TCL_ERROR;
     }
-
     Drawable d = Tk_WindowId(root);
     NSView *rootview = TkMacOSXGetRootControl(d);
     NSWindow *win = [rootview window];
-    int result;
+    NSView *view = [win contentView];
+    NSMenu *menu = (NSMenu *) menuPtr->platformData;
+    NSInteger itemIndex = index;
+    NSInteger numItems = [menu numberOfItems];
+    NSMenuItem *item = nil;
+    NSPoint location = NSMakePoint(x, tkMacOSXZeroScreenHeight - y);
 
     inPostMenu = 1;
-
     result = TkPreprocessMenu(menuPtr);
     if (result != TCL_OK) {
         inPostMenu = 0;
         return result;
     }
+    if (itemIndex >= numItems) {
+    	itemIndex = numItems - 1;
+    }
+    if (itemIndex >= 0) {
+	item = [menu itemAtIndex:itemIndex];
+    }
+    
+    /*
+     * The post commands could have deleted the menu, which means we are dead
+     * and should go away.
+     */
 
-    int oldMode = Tcl_SetServiceMode(TCL_SERVICE_NONE);
-    NSView *view = [win contentView];
-    NSRect frame = NSMakeRect(x + 9, tkMacOSXZeroScreenHeight - y - 9, 1, 1);
-
-    frame.origin = [view convertPoint:
-	    [win tkConvertPointFromScreen:frame.origin] fromView:nil];
+    if (menuPtr->tkwin == NULL) {
+    	return TCL_OK;
+    }
 
-    NSMenu *menu = (NSMenu *) menuPtr->platformData;
-    NSPopUpButtonCell *popUpButtonCell = [[NSPopUpButtonCell alloc]
-	    initTextCell:@"" pullsDown:NO];
-
-    [popUpButtonCell setAltersStateOfSelectedItem:NO];
-    [popUpButtonCell setMenu:menu];
-    [popUpButtonCell selectItem:nil];
-    [popUpButtonCell performClickWithFrame:frame inView:view];
-    [popUpButtonCell release];
-    Tcl_SetServiceMode(oldMode);
+    [menu popUpMenuPositioningItem:item
+			atLocation:[win tkConvertPointFromScreen:location]
+			    inView:view];
     inPostMenu = 0;
     return TCL_OK;
 }
@@ -821,6 +826,109 @@ TkpPostMenu(
 /*
  *----------------------------------------------------------------------
  *
+ * TkpPostTearoffMenu --
+ *
+ *	Tearoff menus are not supported on the Mac.  This placeholder
+ *      function, which is simply a copy of the unix function, posts a
+ *      completely useless window with a black background on the screen. If
+ *      entry is < 0 then the window is positioned so that its top left corner
+ *      is located at the point with screen coordinates (x, y).  Otherwise the
+ *      window position is offset so that top left corner of the specified
+ *      entry would be located at that point, if there actually were a menu.
+ *
+ *      Mac menus steal all mouse or keyboard input from the application until
+ *      the menu is dismissed, with or without a selection, by a mouse or key
+ *      event.  Posting a Mac menu in a regression test will cause the test to
+ *      halt waiting for user input.  This is why the TkpPostMenu function is
+ *      not being used as the placeholder.
+ *
+ * Results:
+ *	None.
+ *
+ * Side effects:
+ *	A useless window is posted.
+ *
+ *----------------------------------------------------------------------
+ */
+
+int
+TkpPostTearoffMenu(
+    Tcl_Interp *interp,		/* The interpreter this menu lives in */
+    TkMenu *menuPtr,		/* The menu we are posting */
+    int x, int y, int index)	/* The screen coordinates where the top left
+				 * corner of the menu, or of the specified
+				 * entry, will be located. */
+{
+    int vRootX, vRootY, vRootWidth, vRootHeight;
+    int result;
+
+    if (index >= menuPtr->numEntries) {
+	index = menuPtr->numEntries - 1;
+    }
+    if (index >= 0) {
+	y -= menuPtr->entries[index]->y;
+    }
+
+    TkActivateMenuEntry(menuPtr, -1);
+    TkRecomputeMenu(menuPtr);
+    result = TkPostCommand(menuPtr);
+    if (result != TCL_OK) {
+    	return result;
+    }
+
+    /*
+     * The post commands could have deleted the menu, which means we are dead
+     * and should go away.
+     */
+
+    if (menuPtr->tkwin == NULL) {
+    	return TCL_OK;
+    }
+
+    /*
+     * Adjust the position of the menu if necessary to keep it visible on the
+     * screen. There are two special tricks to make this work right:
+     *
+     * 1. If a virtual root window manager is being used then the coordinates
+     *    are in the virtual root window of menuPtr's parent; since the menu
+     *    uses override-redirect mode it will be in the *real* root window for
+     *    the screen, so we have to map the coordinates from the virtual root
+     *    (if any) to the real root. Can't get the virtual root from the menu
+     *    itself (it will never be seen by the wm) so use its parent instead
+     *    (it would be better to have an an option that names a window to use
+     *    for this...).
+     * 2. The menu may not have been mapped yet, so its current size might be
+     *    the default 1x1. To compute how much space it needs, use its
+     *    requested size, not its actual size.
+     */
+
+    Tk_GetVRootGeometry(Tk_Parent(menuPtr->tkwin), &vRootX, &vRootY,
+	&vRootWidth, &vRootHeight);
+    vRootWidth -= Tk_ReqWidth(menuPtr->tkwin);
+    if (x > vRootX + vRootWidth) {
+	x = vRootX + vRootWidth;
+    }
+    if (x < vRootX) {
+	x = vRootX;
+    }
+    vRootHeight -= Tk_ReqHeight(menuPtr->tkwin);
+    if (y > vRootY + vRootHeight) {
+	y = vRootY + vRootHeight;
+    }
+    if (y < vRootY) {
+	y = vRootY;
+    }
+    Tk_MoveToplevelWindow(menuPtr->tkwin, x, y);
+    if (!Tk_IsMapped(menuPtr->tkwin)) {
+	Tk_MapWindow(menuPtr->tkwin);
+    }
+    TkWmRestackToplevel((TkWindow *) menuPtr->tkwin, Above, NULL);
+    return TCL_OK;
+}
+
+/*
+ *----------------------------------------------------------------------
+ *
  * TkpSetWindowMenuBar --
  *
  *	Associates a given menu with a window.
@@ -1087,14 +1195,15 @@ void
 TkpComputeStandardMenuGeometry(
     TkMenu *menuPtr)		/* Structure describing menu. */
 {
+    NSSize menuSize = [(NSMenu *)menuPtr->platformData size];
     Tk_Font tkfont, menuFont;
     Tk_FontMetrics menuMetrics, entryMetrics, *fmPtr;
     int modifierCharWidth, menuModifierCharWidth;
     int x, y, modifierWidth, labelWidth, indicatorSpace;
     int windowWidth, windowHeight, accelWidth;
-    int i, j, lastColumnBreak, maxWidth;
+    int i, maxWidth;
     int entryWidth, maxIndicatorSpace, borderWidth, activeBorderWidth;
-    TkMenuEntry *mePtr, *columnEntryPtr;
+    TkMenuEntry *mePtr;
     int haveAccel = 0;
 
     if (menuPtr->tkwin == NULL) {
@@ -1106,7 +1215,7 @@ TkpComputeStandardMenuGeometry(
     Tk_GetPixelsFromObj(NULL, menuPtr->tkwin, menuPtr->activeBorderWidthPtr,
 	    &activeBorderWidth);
     x = y = borderWidth;
-    windowHeight = maxWidth = lastColumnBreak = 0;
+    windowHeight = maxWidth = 0;
     maxIndicatorSpace = 0;
 
     /*
@@ -1133,6 +1242,9 @@ TkpComputeStandardMenuGeometry(
 
     for (i = 0; i < menuPtr->numEntries; i++) {
 	mePtr = menuPtr->entries[i];
+	if (mePtr->type == TEAROFF_ENTRY) {
+	    continue;
+	}
 	if (mePtr->fontPtr == NULL) {
 	    tkfont = menuFont;
 	    fmPtr = &menuMetrics;
@@ -1143,26 +1255,8 @@ TkpComputeStandardMenuGeometry(
 	    fmPtr = &entryMetrics;
 	    modifierCharWidth = ModifierCharWidth(tkfont);
 	}
-
-	if ((i > 0) && mePtr->columnBreak) {
-	    if (maxIndicatorSpace != 0) {
-		maxIndicatorSpace += 2;
-	    }
-	    for (j = lastColumnBreak; j < i; j++) {
-		columnEntryPtr = menuPtr->entries[j];
-		columnEntryPtr->indicatorSpace = maxIndicatorSpace;
-		columnEntryPtr->width = maxIndicatorSpace + maxWidth
-			+ 2 * activeBorderWidth;
-		columnEntryPtr->x = x;
-		columnEntryPtr->entryFlags &= ~ENTRY_LAST_COLUMN;
-	    }
-	    x += maxIndicatorSpace + maxWidth + 2 * activeBorderWidth;
-	    maxWidth = maxIndicatorSpace = 0;
-	    lastColumnBreak = i;
-	    y = borderWidth;
-	}
 	accelWidth = modifierWidth = indicatorSpace = 0;
-	if (mePtr->type == SEPARATOR_ENTRY || mePtr->type == TEAROFF_ENTRY) {
+	if (mePtr->type == SEPARATOR_ENTRY) {
 	    mePtr->height = menuSeparatorHeight;
 	} else {
 	    /*
@@ -1176,16 +1270,16 @@ TkpComputeStandardMenuGeometry(
 
 	    NSMenuItem *menuItem = (NSMenuItem *) mePtr->platformEntryData;
 	    int haveImage = 0, width = 0, height = 0;
-
 	    if (mePtr->image) {
 		Tk_SizeOfImage(mePtr->image, &width, &height);
 		haveImage = 1;
+		height += 2; /* tweak */
 	    } else if (mePtr->bitmapPtr) {
 		Pixmap bitmap = Tk_GetBitmapFromObj(menuPtr->tkwin,
 			mePtr->bitmapPtr);
-
 		Tk_SizeOfBitmap(menuPtr->display, bitmap, &width, &height);
 		haveImage = 1;
+		height += 2; /* tweak */
 	    }
 	    if (!haveImage || (mePtr->compound != COMPOUND_NONE)) {
 		NSAttributedString *attrTitle = [menuItem attributedTitle];
@@ -1197,11 +1291,8 @@ TkpComputeStandardMenuGeometry(
 		    size = [[menuItem title] sizeWithAttributes:
 			TkMacOSXNSFontAttributesForFont(tkfont)];
 		}
-		size.width += menuTextLeadingEdgeMargin +
-			menuTextTrailingEdgeMargin;
-		if (size.height < fmPtr->linespace) {
-		    size.height = fmPtr->linespace;
-		}
+		size.width += menuTextLeadingEdgeMargin + menuTextTrailingEdgeMargin;
+		size.height -= 1; /* tweak */
 		if (haveImage && (mePtr->compound != COMPOUND_NONE)) {
 		    int margin = width + menuIconTrailingEdgeMargin;
 
@@ -1217,9 +1308,11 @@ TkpComputeStandardMenuGeometry(
 		    height = size.height;
 		}
 	    }
+	    else {
+		/* image only. */
+	    }
 	    labelWidth = width + menuItemExtraWidth;
 	    mePtr->height = height + menuItemExtraHeight;
-
 	    if (mePtr->type == CASCADE_ENTRY) {
 		modifierWidth = modifierCharWidth;
 	    } else if (mePtr->accelLength == 0) {
@@ -1250,30 +1343,18 @@ TkpComputeStandardMenuGeometry(
 	    if (entryWidth > maxWidth) {
 		maxWidth = entryWidth;
 	    }
+	    menuPtr->entries[i]->width = entryWidth;
 	    mePtr->height += 2 * activeBorderWidth;
 	}
+	mePtr->x = x;
 	mePtr->y = y;
 	y += menuPtr->entries[i]->height + borderWidth;
-	if (y > windowHeight) {
-	    windowHeight = y;
-	}
-    }
-
-    for (j = lastColumnBreak; j < menuPtr->numEntries; j++) {
-	columnEntryPtr = menuPtr->entries[j];
-	columnEntryPtr->indicatorSpace = maxIndicatorSpace;
-	columnEntryPtr->width = maxIndicatorSpace + maxWidth
-		+ 2 * activeBorderWidth;
-	columnEntryPtr->x = x;
-	columnEntryPtr->entryFlags |= ENTRY_LAST_COLUMN;
     }
-    windowWidth = x + maxIndicatorSpace + maxWidth
-	    + 2 * activeBorderWidth + borderWidth;
-    windowHeight += borderWidth;
-
+    windowWidth = menuSize.width;
     if (windowWidth <= 0) {
 	windowWidth = 1;
     }
+    windowHeight = menuSize.height;
     if (windowHeight <= 0) {
 	windowHeight = 1;
     }
diff --git a/macosx/tkMacOSXMenubutton.c b/macosx/tkMacOSXMenubutton.c
index 1acefe5..b2b4b76 100644
--- a/macosx/tkMacOSXMenubutton.c
+++ b/macosx/tkMacOSXMenubutton.c
@@ -47,17 +47,23 @@ typedef struct MacMenuButton {
 } MacMenuButton;
 
 /*
- * Forward declarations for procedures defined later in this file:
+ * Forward declarations for static functions defined later in this file:
  */
 
 static void MenuButtonEventProc(ClientData clientData, XEvent *eventPtr);
-static void MenuButtonBackgroundDrawCB ( MacMenuButton *ptr, SInt16 depth, Boolean isColorDev);
-static void MenuButtonContentDrawCB ( ThemeButtonKind kind, const HIThemeButtonDrawInfo * info, MacMenuButton *ptr, SInt16 depth, Boolean isColorDev);
+static void MenuButtonBackgroundDrawCB (MacMenuButton *ptr, SInt16 depth,
+					Boolean isColorDev);
+static void MenuButtonContentDrawCB (ThemeButtonKind kind,
+				     const HIThemeButtonDrawInfo * info,
+				     MacMenuButton *ptr, SInt16 depth,
+				     Boolean isColorDev);
 static void MenuButtonEventProc ( ClientData clientData, XEvent *eventPtr);
-static void TkMacOSXComputeMenuButtonParams (TkMenuButton * butPtr, ThemeButtonKind* btnkind, HIThemeButtonDrawInfo* drawinfo);
-static int TkMacOSXComputeMenuButtonDrawParams (TkMenuButton * butPtr, DrawParams * dpPtr);
-static void TkMacOSXDrawMenuButton (MacMenuButton *butPtr,
-       GC gc, Pixmap pixmap);
+static void TkMacOSXComputeMenuButtonParams (TkMenuButton * butPtr,
+					     ThemeButtonKind* btnkind,
+					     HIThemeButtonDrawInfo* drawinfo);
+static void TkMacOSXComputeMenuButtonDrawParams (TkMenuButton * butPtr,
+						 DrawParams * dpPtr);
+static void TkMacOSXDrawMenuButton (MacMenuButton *butPtr, GC gc, Pixmap pixmap);
 static void DrawMenuButtonImageAndText(TkMenuButton* butPtr);
 
 /*
@@ -70,11 +76,45 @@ Tk_ClassProcs tkpMenubuttonClass = {
     TkMenuButtonWorldChanged,	/* worldChangedProc */
 };
 
+/*
+ * We use Apple's Pop-Up Button widget to represent the Tk Menubutton.
+ * However, we do not use the NSPopUpButton class for this control.  Instead we
+ * render the Pop-Up Button using the HITheme library.  This imposes some
+ * constraints on what can be done.  The HITheme renderer allows only specific
+ * dimensions for the button.
+ *
+ * The HITheme library allows drawing a Pop-Up Button with an arbitrary bounds
+ * rectangle.  However the button is always drawn as a rounded box which is 22
+ * pixels high.  If the bounds rectangle is less than 22 pixels high, the
+ * button is drawn at the top of the rectangle and the bottom of the button is
+ * clipped away.  So we set a minimum height of 22 pixels for a Menubutton.  If
+ * the bounds rectangle is more than 22 pixels high, then the button is drawn
+ * centered vertically in the bounds rectangle.
+ *
+ * The content rectangle of the button is inset by 14 pixels on the left and 28
+ * pixels on the right.  The rightmost part of the button contains the blue
+ * double-arrow symbol which is 28 pixels wide.
+ *
+ * To maintain compatibility with code that runs on multiple operating systems,
+ * the width and height of the content rectangle includes the borderWidth, the
+ * highlightWidth and the padX and padY dimensions of the Menubutton.  However,
+ * to be consistent with the standard Apple appearance, the content is always
+ * be drawn at the left side of the content rectangle.  All of the excess space
+ * appears on the right side of the content, and the anchor property is
+ * ignored.  The easiest way to comply with Apple's Human Interface Guidelines
+ * would be to set bd = highlightthickness = padx = 0 and to specify an
+ * explicit width for the button.  Apple also recommends using the same width
+ * for all Pop-Up Buttons in a given window.
+ */
+
+#define LEFT_INSET 8
+#define RIGHT_INSET 28
+#define MIN_HEIGHT 22
 
 /*
  *----------------------------------------------------------------------
  *
- * TkpCreateMenuButton --
+ * TkpCreateMenuButton  --
  *
  *	Allocate a new TkMenuButton structure.
  *
@@ -93,13 +133,12 @@ TkpCreateMenuButton(
 {
     MacMenuButton *mbPtr = (MacMenuButton *) ckalloc(sizeof(MacMenuButton));
 
-    Tk_CreateEventHandler(tkwin, ActivateMask,
-	    MenuButtonEventProc, (ClientData) mbPtr);
+    Tk_CreateEventHandler(tkwin, ActivateMask, MenuButtonEventProc,
+			  (ClientData) mbPtr);
     mbPtr->flags = FIRST_DRAW;
     mbPtr->btnkind = kThemePopupButton;
     bzero(&mbPtr->drawinfo, sizeof(mbPtr->drawinfo));
     bzero(&mbPtr->lastdrawinfo, sizeof(mbPtr->lastdrawinfo));
-
     return (TkMenuButton *) mbPtr;
 }
 
@@ -165,12 +204,13 @@ TkpDisplayMenuButton(
  * TkpDestroyMenuButton --
  *
  *	Free data structures associated with the menubutton control.
+ *      This is a no-op on the Mac.
  *
  * Results:
  *	None.
  *
  * Side effects:
- *	Restores the default control state.
+ *	None.
  *
  *----------------------------------------------------------------------
  */
@@ -204,15 +244,12 @@ TkpComputeMenuButtonGeometry(butPtr)
     register TkMenuButton *butPtr;	/* Widget record for menu button. */
 {
     int width, height, avgWidth, haveImage = 0, haveText = 0;
-    MacMenuButton *mbPtr = (MacMenuButton*)butPtr;
     int txtWidth, txtHeight;
     Tk_FontMetrics fm;
-    DrawParams drawParams;
-    int paddingx = 0;
-    int paddingy = 0;
+    int highlightWidth = butPtr->highlightWidth > 0 ? butPtr->highlightWidth : 0;
 
     /*
-     * First figure out the size of the contents of the button.
+     * First compute the size of the contents of the button.
      */
 
     width = 0;
@@ -221,8 +258,6 @@ TkpComputeMenuButtonGeometry(butPtr)
     txtHeight = 0;
     avgWidth = 0;
 
-    TkMacOSXComputeMenuButtonParams(butPtr, &mbPtr->btnkind, &mbPtr->drawinfo);
-
     if (butPtr->image != NULL) {
         Tk_SizeOfImage(butPtr->image, &width, &height);
         haveImage = 1;
@@ -231,17 +266,16 @@ TkpComputeMenuButtonGeometry(butPtr)
         haveImage = 1;
     }
 
-    if (haveImage == 0 || butPtr->compound != COMPOUND_NONE) {
+    if (butPtr->text && strlen(butPtr->text) > 0) {
+	haveText = 1;
         Tk_FreeTextLayout(butPtr->textLayout);
         butPtr->textLayout = Tk_ComputeTextLayout(butPtr->tkfont,
                 butPtr->text, -1, butPtr->wrapLength,
                 butPtr->justify, 0, &butPtr->textWidth, &butPtr->textHeight);
-
         txtWidth = butPtr->textWidth;
         txtHeight = butPtr->textHeight;
         avgWidth = Tk_TextWidth(butPtr->tkfont, "0", 1);
         Tk_GetFontMetrics(butPtr->tkfont, &fm);
-        haveText = (txtWidth != 0 && txtHeight != 0);
     }
 
     /*
@@ -251,7 +285,7 @@ TkpComputeMenuButtonGeometry(butPtr)
      * image, because otherwise it is not really a compound button.
      */
 
-    if (butPtr->compound != COMPOUND_NONE && haveImage && haveText) {
+    if (haveImage && haveText) {
         switch ((enum compound) butPtr->compound) {
             case COMPOUND_TOP:
             case COMPOUND_BOTTOM: {
@@ -293,76 +327,29 @@ TkpComputeMenuButtonGeometry(butPtr)
         }
 
     } else {
-        if (haveImage) {
+        if (haveImage) { /* Image only */
             if (butPtr->width > 0) {
                 width = butPtr->width;
             }
             if (butPtr->height > 0) {
                 height = butPtr->height;
             }
-        } else {
+        } else { /* Text only */
             width = txtWidth;
             height = txtHeight;
             if (butPtr->width > 0) {
-                width = butPtr->width * avgWidth;
+                width = butPtr->width * avgWidth + 2*butPtr->padX;
             }
             if (butPtr->height > 0) {
-                height = butPtr->height * fm.linespace;
+                height = butPtr->height * fm.linespace + 2*butPtr->padY;
             }
         }
     }
-    width  += 2 * butPtr->padX - 2;
-    height += 2 * butPtr->padY - 2;
-
-    /*Add padding for button arrows.*/
-    width += 22;
-
-    /*
-     * Now figure out the size of the border decorations for the button.
-     */
-
-    if (butPtr->highlightWidth < 0) {
-        butPtr->highlightWidth = 0;
-    }
-    butPtr->inset = 0;
-    butPtr->inset += butPtr->highlightWidth;
-
-    TkMacOSXComputeMenuButtonDrawParams(butPtr,&drawParams);
-
-        HIRect tmpRect;
-	HIRect contBounds;
-
-	tmpRect = CGRectMake(0, 0, width, height);
-
-	HIThemeGetButtonContentBounds(&tmpRect, &mbPtr->drawinfo, &contBounds);
-
-
-
-        /* If the content region has a minimum height, match it. */
-        if (height < contBounds.size.height) {
-	  height = contBounds.size.height;
-        }
-
-        /* If the content region has a minimum width, match it. */
-        if (width < contBounds.size.width) {
-	  width = contBounds.size.width;
-        }
-
-        /* Pad to fill difference between content bounds and button bounds. */
-        paddingx = tmpRect.origin.x - contBounds.origin.x;
-        paddingy = tmpRect.origin.y - contBounds.origin.y;
-
-    if (paddingx > 0) {
-        width += paddingx;
-    }
-    if (paddingy > 0) {
-        height += paddingy;
-    }
-
-    width += butPtr->inset*2;
-    height += butPtr->inset*2;
-
-
+    
+    butPtr->inset = highlightWidth + butPtr->borderWidth;
+    width += LEFT_INSET + RIGHT_INSET + 2*butPtr->inset;
+    height += 2*butPtr->inset;
+    height = height < MIN_HEIGHT ? MIN_HEIGHT : height;
     Tk_GeometryRequest(butPtr->tkwin, width, height);
     Tk_SetInternalBorder(butPtr->tkwin, butPtr->inset);
 }
@@ -427,8 +414,8 @@ DrawMenuButtonImageAndText(
         pressed = 1;
     }
 
-  haveText = (butPtr->textWidth != 0 && butPtr->textHeight != 0);
-   if (butPtr->compound != COMPOUND_NONE && haveImage && haveText) {
+    haveText = (butPtr->textWidth != 0 && butPtr->textHeight != 0);
+    if (butPtr->compound != COMPOUND_NONE && haveImage && haveText) {
         int x = 0;
         int y = 0;
         textXOffset = 0;
@@ -446,8 +433,8 @@ DrawMenuButtonImageAndText(
                     imageYOffset = butPtr->textHeight + butPtr->padY;
                 }
                 fullHeight = height + butPtr->textHeight + butPtr->padY;
-                fullWidth = (width > butPtr->textWidth ? width :
-                        butPtr->textWidth);
+                fullWidth = (width > butPtr->textWidth ?
+			     width : butPtr->textWidth);
                 textXOffset = (fullWidth - butPtr->textWidth)/2;
                 imageXOffset = (fullWidth - width)/2;
                 break;
@@ -489,10 +476,10 @@ DrawMenuButtonImageAndText(
 	}
 
         TkComputeAnchor(butPtr->anchor, tkwin,
-                butPtr->padX + butPtr->borderWidth,
-                butPtr->padY + butPtr->borderWidth,
+                butPtr->padX + butPtr->inset,
+                butPtr->padY + butPtr->inset,
                 fullWidth, fullHeight, &x, &y);
-        imageXOffset += x;
+        imageXOffset = LEFT_INSET;
         imageYOffset += y;
         textYOffset -= 1;
 
@@ -517,36 +504,32 @@ DrawMenuButtonImageAndText(
                 butPtr->underline);
     } else {
         if (haveImage) {
-            int x = 0;
-            int y;
+            int x, y;
             TkComputeAnchor(butPtr->anchor, tkwin,
                     butPtr->padX + butPtr->borderWidth,
                     butPtr->padY + butPtr->borderWidth,
                     width, height, &x, &y);
-	        imageXOffset += x;
-	    	imageYOffset += y;
-
-               if (butPtr->image != NULL) {
-		     Tk_RedrawImage(butPtr->image, 0, 0, width, height,
-		         pixmap, imageXOffset, imageYOffset);
+	    imageXOffset = LEFT_INSET;
+	    imageYOffset += y;
+	    if (butPtr->image != NULL) {
+		Tk_RedrawImage(butPtr->image, 0, 0, width, height,
+			       pixmap, imageXOffset, imageYOffset);
             } else {
                 XSetClipOrigin(butPtr->display, dpPtr->gc, x, y);
                 XCopyPlane(butPtr->display, butPtr->bitmap,
-                        pixmap, dpPtr->gc,
-                        0, 0, (unsigned int) width,
-                        (unsigned int) height,
-                        imageXOffset, imageYOffset, 1);
+			   pixmap, dpPtr->gc,
+			   0, 0, (unsigned int) width,
+			   (unsigned int) height,
+			   imageXOffset, imageYOffset, 1);
                 XSetClipOrigin(butPtr->display, dpPtr->gc, 0, 0);
             }
         } else {
-	  /*Move x back by eight pixels to give the menubutton arrows room.*/
-	  int x = 0;
-	  int y;
-	  textXOffset = 8;
+	    int x, y;
+	    textXOffset = LEFT_INSET;
 	    TkComputeAnchor(butPtr->anchor, tkwin, butPtr->padX, butPtr->padY,
 			    butPtr->textWidth, butPtr->textHeight, &x, &y);
 	    Tk_DrawTextLayout(butPtr->display, pixmap, dpPtr->gc,
-			      butPtr->textLayout, x - textXOffset, y, 0, -1);
+			      butPtr->textLayout, textXOffset, y, 0, -1);
 	    y += butPtr->textHeight/2;
 	  }
    }
@@ -578,7 +561,6 @@ TkMacOSXDrawMenuButton(
                            * the bevel button */
     Pixmap pixmap)        /* The pixmap we are drawing into - needed
                            * for the bevel button */
-
 {
     TkMenuButton * butPtr = ( TkMenuButton *)mbPtr;
     TkWindow * winPtr;
@@ -591,10 +573,9 @@ TkMacOSXDrawMenuButton(
 
     TkMacOSXComputeMenuButtonParams(butPtr, &mbPtr->btnkind, &mbPtr->drawinfo);
 
-    cntrRect = CGRectMake(winPtr->privatePtr->xOff, winPtr->privatePtr->yOff, Tk_Width(butPtr->tkwin),Tk_Height(butPtr->tkwin));
-
-     cntrRect = CGRectInset(cntrRect,  butPtr->inset, butPtr->inset);
-
+    cntrRect = CGRectMake(winPtr->privatePtr->xOff, winPtr->privatePtr->yOff,
+			  Tk_Width(butPtr->tkwin),
+			  Tk_Height(butPtr->tkwin));
 
     if (useNewerHITools == 1) {
         HIRect contHIRec;
@@ -617,17 +598,15 @@ TkMacOSXDrawMenuButton(
             hiinfo.animation.time.start = hiinfo.animation.time.current;
         }
 
-        HIThemeDrawButton(&cntrRect, &hiinfo, dc.context, kHIThemeOrientationNormal, &contHIRec);
-
+        HIThemeDrawButton(&cntrRect, &hiinfo, dc.context,
+			  kHIThemeOrientationNormal, &contHIRec);
 	TkMacOSXRestoreDrawingContext(&dc);
-
-        MenuButtonContentDrawCB( mbPtr->btnkind, &mbPtr->drawinfo, (MacMenuButton *)mbPtr, 32, true);
+        MenuButtonContentDrawCB( mbPtr->btnkind, &mbPtr->drawinfo,
+				 (MacMenuButton *)mbPtr, 32, true);
     } else {
 	if (!TkMacOSXSetupDrawingContext(pixmap, dpPtr->gc, 1, &dc)) {
 	    return;
 	}
-
-
 	TkMacOSXRestoreDrawingContext(&dc);
     }
     mbPtr->lastdrawinfo = mbPtr->drawinfo;
@@ -696,8 +675,7 @@ MenuButtonContentDrawCB (
     if (tkwin == NULL || !Tk_IsMapped(tkwin)) {
         return;
     }
-
-    DrawMenuButtonImageAndText( butPtr);
+    DrawMenuButtonImageAndText(butPtr);
 }
 
 /*
@@ -761,19 +739,18 @@ MenuButtonEventProc(
  */
 
 static void
-TkMacOSXComputeMenuButtonParams(TkMenuButton * butPtr, ThemeButtonKind* btnkind, HIThemeButtonDrawInfo *drawinfo)
+TkMacOSXComputeMenuButtonParams(
+    TkMenuButton * butPtr,
+    ThemeButtonKind* btnkind,
+    HIThemeButtonDrawInfo *drawinfo)
 {
     MacMenuButton *mbPtr = (MacMenuButton *)butPtr;
 
-    if (butPtr->image || butPtr->bitmap) {
+    if (butPtr->image || butPtr->bitmap || butPtr->text) {
 	/* TODO: allow for Small and Mini menubuttons. */
 	*btnkind = kThemePopupButton;
-    } else {
-        if (!butPtr->text || !*butPtr->text) {
-            *btnkind = kThemeArrowButton;
-        } else {
-            *btnkind = kThemePopupButton;
-        }
+    } else { /* This should never happen. */
+	*btnkind = kThemeArrowButton;
     }
 
     drawinfo->value = kThemeButtonOff;
@@ -812,24 +789,25 @@ TkMacOSXComputeMenuButtonParams(TkMenuButton * butPtr, ThemeButtonKind* btnkind,
  *
  * TkMacOSXComputeMenuButtonDrawParams --
  *
- *        This procedure computes the various parameters used
- *        when drawing a button
- *      These are determined by the various tk button parameters
+ *        This procedure selects an appropriate drawing context for
+ *        drawing a menubutton.
  *
  * Results:
- *        1 if control will be used, 0 otherwise.
+ *        None.
  *
  * Side effects:
- *        Sets the button draw parameters
+ *        Sets the button draw parameters.
  *
  *----------------------------------------------------------------------
  */
 
-static int
-TkMacOSXComputeMenuButtonDrawParams(TkMenuButton * butPtr, DrawParams * dpPtr)
+static void
+TkMacOSXComputeMenuButtonDrawParams(
+    TkMenuButton * butPtr,
+    DrawParams * dpPtr)
 {
-    dpPtr->hasImageOrBitmap = ((butPtr->image != NULL)
-            || (butPtr->bitmap != None));
+    dpPtr->hasImageOrBitmap = ((butPtr->image != NULL) ||
+			       (butPtr->bitmap != None));
     dpPtr->border = butPtr->normalBorder;
     if ((butPtr->state == STATE_DISABLED) && (butPtr->disabledFg != NULL)) {
         dpPtr->gc = butPtr->disabledGC;
@@ -839,8 +817,6 @@ TkMacOSXComputeMenuButtonDrawParams(TkMenuButton * butPtr, DrawParams * dpPtr)
     } else {
         dpPtr->gc = butPtr->normalTextGC;
     }
-
-    return 1;
 }
 
 /*