Bug 485118 - Always set the right minimum size for the scrollbar thumb. r=josh
authorMarkus Stange <mstange@themasta.com>
Tue, 06 Oct 2009 16:02:50 +1300
changeset 33516 b8529fc4706065f646149cbc65d83d75eae1c9bd
parent 33515 f47c8163a2a5fd2de3af40a469d08f700fb75e6b
child 33517 21ca51ccd2a9ce51c343f4942527fcc1b0993688
push id9567
push usermstange@themasta.com
push dateTue, 06 Oct 2009 20:39:34 +0000
treeherdermozilla-central@b8529fc47060 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjosh
bugs485118
milestone1.9.3a1pre
Bug 485118 - Always set the right minimum size for the scrollbar thumb. r=josh
widget/src/cocoa/nsNativeThemeCocoa.mm
widget/tests/Makefile.in
widget/tests/test_bug485118.xul
--- a/widget/src/cocoa/nsNativeThemeCocoa.mm
+++ b/widget/src/cocoa/nsNativeThemeCocoa.mm
@@ -2106,16 +2106,19 @@ nsNativeThemeCocoa::GetWidgetOverflow(ns
       aOverflowRect->Inflate(m);
       return PR_TRUE;
     }
   }
 
   return PR_FALSE;
 }
 
+static const PRInt32 kRegularScrollbarThumbMinSize = 22;
+static const PRInt32 kSmallScrollbarThumbMinSize = 19;
+
 NS_IMETHODIMP
 nsNativeThemeCocoa::GetMinimumWidgetSize(nsIRenderingContext* aContext,
                                          nsIFrame* aFrame,
                                          PRUint8 aWidgetType,
                                          nsIntSize* aResult,
                                          PRBool* aIsOverridable)
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT;
@@ -2217,57 +2220,29 @@ nsNativeThemeCocoa::GetMinimumWidgetSize
     {
       SInt32 scrollbarWidth = 0;
       ::GetThemeMetric(kThemeMetricSmallScrollBarWidth, &scrollbarWidth);
       aResult->SizeTo(scrollbarWidth, scrollbarWidth);
       *aIsOverridable = PR_FALSE;
       break;
     }
 
-    // Get the rect of the thumb from HITheme, so we can return it to Gecko, which has different ideas about
-    // how big the thumb should be. This is kind of a hack.
     case NS_THEME_SCROLLBAR_THUMB_HORIZONTAL:
     case NS_THEME_SCROLLBAR_THUMB_VERTICAL:
     {
-      // Find our parent scrollbar frame. If we can't, abort.
+      // Find our parent scrollbar frame in order to find out whether we're in
+      // a small or a large scrollbar.
       nsIFrame *scrollbarFrame = GetParentScrollbarFrame(aFrame);
-      if (!scrollbarFrame) return NS_ERROR_FAILURE;
-
-      nsRect scrollbarRect = scrollbarFrame->GetRect();      
-      *aIsOverridable = PR_FALSE;
-
-      if (scrollbarRect.IsEmpty()) {
-        // just return (0,0)
-        return NS_OK;
-      }
+      if (!scrollbarFrame)
+        return NS_ERROR_FAILURE;
 
-      // We need to get the device context to convert from app units :(
-      nsCOMPtr<nsIDeviceContext> dctx;
-      aContext->GetDeviceContext(*getter_AddRefs(dctx));
-      PRInt32 p2a = dctx->AppUnitsPerDevPixel();
-      CGRect macRect = CGRectMake(NSAppUnitsToIntPixels(scrollbarRect.x, p2a),
-                                  NSAppUnitsToIntPixels(scrollbarRect.y, p2a),
-                                  NSAppUnitsToIntPixels(scrollbarRect.width, p2a),
-                                  NSAppUnitsToIntPixels(scrollbarRect.height, p2a));
-
-      // False here means not to get scrollbar button state information.
-      HIThemeTrackDrawInfo tdi;
-      GetScrollbarDrawInfo(tdi, scrollbarFrame, macRect.size, PR_FALSE);
-
-      HIRect thumbRect;
-      ::HIThemeGetTrackPartBounds(&tdi, kControlIndicatorPart, &thumbRect);
-
-      // HITheme is just lying to us, I guess...
-      PRInt32 thumbAdjust = ((scrollbarFrame->GetStyleDisplay()->mAppearance == NS_THEME_SCROLLBAR_SMALL) ?
-                             2 : 4);
-
-      if (aWidgetType == NS_THEME_SCROLLBAR_THUMB_VERTICAL)
-        aResult->SizeTo(nscoord(thumbRect.size.width), nscoord(thumbRect.size.height - thumbAdjust));
-      else
-        aResult->SizeTo(nscoord(thumbRect.size.width - thumbAdjust), nscoord(thumbRect.size.height));
+      PRBool isSmall = (scrollbarFrame->GetStyleDisplay()->mAppearance == NS_THEME_SCROLLBAR_SMALL);
+      PRBool isHorizontal = (aWidgetType == NS_THEME_SCROLLBAR_THUMB_HORIZONTAL);
+      PRInt32& minSize = isHorizontal ? aResult->width : aResult->height;
+      minSize = isSmall ? kSmallScrollbarThumbMinSize : kRegularScrollbarThumbMinSize;
       break;
     }
 
     case NS_THEME_SCROLLBAR:
     case NS_THEME_SCROLLBAR_TRACK_VERTICAL:
     case NS_THEME_SCROLLBAR_TRACK_HORIZONTAL:
     {
       // yeah, i know i'm cheating a little here, but i figure that it
--- a/widget/tests/Makefile.in
+++ b/widget/tests/Makefile.in
@@ -66,16 +66,17 @@ include $(topsrcdir)/config/rules.mk
 
 ifeq ($(MOZ_WIDGET_TOOLKIT),cocoa)
 _TEST_FILES += native_menus_window.xul \
                test_native_menus.xul \
                native_mouse_mac_window.xul \
                test_native_mouse_mac.xul \
                test_bug428405.xul \
                test_bug466599.xul \
+               test_bug485118.xul \
                test_platform_colors.xul \
                $(NULL)
 endif
 
 ifeq ($(MOZ_WIDGET_TOOLKIT),windows)
 ifneq ($(OS_ARCH), WINCE)
 _TEST_FILES  += taskbar_previews.xul \
 		taskbar_progress.xul \
new file mode 100644
--- /dev/null
+++ b/widget/tests/test_bug485118.xul
@@ -0,0 +1,74 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css"
+                 type="text/css"?>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=485118
+-->
+<window title="Mozilla Bug 485118"
+  xmlns:html="http://www.w3.org/1999/xhtml"
+  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+
+  <title>Test for Bug 485118</title>
+  <script type="application/javascript" 
+   src="chrome://mochikit/content/MochiKit/packed.js"></script>
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js" />
+
+<body  xmlns="http://www.w3.org/1999/xhtml">
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+</pre>
+</body>
+
+<hbox height="300">
+  <vbox width="300">
+    <scrollbar orient="horizontal"
+               maxpos="10000"
+               pageincrement="1"
+               id="horizontal"/>
+    <scrollbar orient="horizontal"
+               maxpos="10000"
+               pageincrement="1"
+               style="-moz-appearance: scrollbar-small;"
+               id="horizontalSmall"/>
+    <hbox flex="1">
+      <scrollbar orient="vertical"
+                 maxpos="10000"
+                 pageincrement="1"
+                 id="vertical"/>
+      <scrollbar orient="vertical"
+                 maxpos="10000"
+                 pageincrement="1"
+                 style="-moz-appearance: scrollbar-small;"
+                 id="verticalSmall"/>
+      <spacer flex="1"/>
+    </hbox>
+  </vbox>
+</hbox>
+
+<script class="testbody" type="application/javascript">
+<![CDATA[
+
+SimpleTest.waitForExplicitFinish();
+
+SimpleTest.executeSoon(function start() {
+  ["horizontal", "vertical"].forEach(function (orient) {
+    ["", "Small"].forEach(function (size) {
+      var elem = document.getElementById(orient + size);
+      var thumbRect = document.getAnonymousElementByAttribute(elem, 'sbattr', 'scrollbar-thumb').getBoundingClientRect();
+      var sizeToCheck = orient == "horizontal" ? "width" : "height";
+      var expectedSize = size == "Small" ? 19 : 22;
+      is(thumbRect[sizeToCheck], expectedSize, size + " scrollbar has wrong minimum " + sizeToCheck);
+    });
+  });
+  SimpleTest.finish();
+});
+
+
+]]>
+</script>
+
+</window>