423780 - Black border around scrollbars when using -moz-appearance: scrollbar

Status: RESOLVED WORKSFORME

(Core :: Graphics, defect)

Description

[KLB]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008031807 Minefield/3.0b5pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008031807 Minefield/3.0b5pre

As a result of the decision to replace "nativescrollbars.css" and "xulscrollbars.css" with "scrollbars.css" a bad side effect for theme developers is trying to get scrollbars to work correctly across OSes.  In particular, Mac Firefox seems to want the style rule "-moz-appearance: scrollbar" for the object "scrollbar".  The problem is that when this rule is used on WinXP with the default "Windows XP Style" there is a stray 1px black border around all scrollbars that can not be removed using style rules like "border:none !important".

This problem does not appear in WinXP using "Windows Classic Style".

This black border needs to be removed so that theme developers can use "-moz-appearance: scrollbar" when making a cross OS compatible theme.

Reproducible: Always

Steps to Reproduce:
1. Apply the following style rule to userChrome.css or custom theme:

scrollbar {
  -moz-appearance: scrollbar;
}

2. Set WinXP desktop style to "Windows XP Style"

Actual Results:  
Black border around scrollbars

Expected Results:  
No border around scrollbars.

Comment 1

[KLB]

After a thorough review of the Mac and Windows version of scrollbars.css I figured out that the default Windows theme changed the old xulscrollbars.css to scrollbars.css where as Mac's default theme changed the old nativescrollbars.css to scrollbars.css.  Using the old nativescrollbars.css as scrollbar.css on Windows would work if the stray black border issue reported in this bug report was fixed.  

Fixing this bug would make it possible to have one version of scrollbars.css to support all OSes.  This in turn would make cross OS theme development much easier.

Comment 2

[KLB]

Attachment: This screen capture demonstrates the black border when the style instruction "-moz-appearance: scrollbar" is used on WinXP with "Windos XP Style" desktop (as is needed for Mac OS/X).

This screen capture demonstrates the black border when the style instruction "-moz-appearance: scrollbar" is used on WinXP with "Windos XP Style" desktop (as is needed for Mac OS/X). The screen capture was captured using v3.0b5pre ( specifically "Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008032304 Minefield/3.0b5pre"), however, it also exists in v3.0b3 and v3.0b4).

Again eliminating this bug would make it possible to feed a single custom theme JAR file to all OSes, which would ease theme development.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

--> Core::GFX, blocking1.9? set though I would suggest that this doesn't really block, though a safe patch would be accepted.

Comment 4

[KLB]

If I knew where to look I could at least try to help figure out exactly what piece of code is causing the border.  If someone could just point me in the right direction, I'd do what I can to help.

Comment 5

[Neil Deakin]

Likely something in widget/src/windows/nsNativeThemeWin.cpp

Comment 6

[KLB]

I don't know C++ or that kind of programming, but using the the programming skills I do have to look for potential areas of interest, the one section that caught my eye is in "mozilla\widget\src\windows\nsNativeThemeWin.cpp" starting at line 1676.  The comment states:

  // Call GetSystemMetrics to determine size for WinXP scrollbars
  // (GetThemeSysSize API returns the optimal size for the theme, but 
  //  Windows appears to always use metrics when drawing standard scrollbars)

Directly below this statement on lines 1680 through 1689 there is a case statement that lists all scollbar related "-moz-appearance" related declarations (e.g. "scrollbartrack-horizontal") except "scrollbar" or more precisely "NS_THEME_SCROLLBAR".  

Could it be so simple as needing to add the line "case NS_THEME_SCROLLBAR:" to that case statement?

Comment 7

[KLB]

It would also appear that "case NS_THEME_SCROLLBAR:" is also missing from a case statement in the same file which is located on lines 1948 through 1996, in a case statement at lines 2334 through 2364, and again in a case statement at lines 2962 through 2995.

Lines 2334 through 2364 are particularly interesting because they contain the comment:
   // these don't use DrawFrameControl

Comment 8

[KLB]

I notice there has been no activity on this bug report in almost 16 months. Was any progress made in fixing the black border issue around scrollbars on Windows XP when using the style instruction "-moz-appearance:scrollbar"?

Comment 9

[[e-mail]]

It's worse than that because Windows actually uses scrollbartrack-horizontal or scrollbartrack-vertical for its scrollbars...

Comment 10

[Markus Stange [:mstange]]

Yeah, but IIUC adding those -moz-appearances to the cross-OS styles doesn't hurt because they're ignored on Mac.

Comment 11

[[e-mail]]

But you can't actually add them, since they're applied to the same element. The only solutions would seem to me to be a) make -moz-appearance: scrollbar; select the appropriate substyle on Windows/Linux b) make -moz-appearance: scrollbartrack-horizontal/vertical work on Mac.

Comment 12

[Markus Stange [:mstange]]

Attachment: patch (untested)

Does this fix the problem?

Comment 13

[[e-mail]]

Comment on attachment 439022 [details] [diff] [review]
patch (untested)

(still untested!)

>+    case NS_THEME_SCROLLBAR:
>+    case NS_THEME_SCROLLBAR_SMALL:
>     case NS_THEME_SCROLLBAR_TRACK_HORIZONTAL:
>     case NS_THEME_SCROLLBAR_TRACK_VERTICAL: {
>-      aPart = (aWidgetType == NS_THEME_SCROLLBAR_TRACK_HORIZONTAL) ?
>+      aPart = (IsHorizontalScrollbar(aFrame, aWidgetType) ?
>               SP_TRACKSTARTHOR : SP_TRACKSTARTVERT;
IMHO this is confusing, and this should just be split up into three cases:
    case NS_THEME_SCROLLBAR:
    case NS_THEME_SCROLLBAR_SMALL:
      aPart = aFrame &&
              aFrame->GetContent()->AttrValueIs(kNameSpaceID_None,
                                                nsWidgetAtoms::orient,
                                                nsWidgetAtoms::vertical,
                                                eCaseMatters) ?
          SP_TRACKSTARTVERT : SP_TRACKSTARTHOR;
      aState = TS_NORMAL;
      return NS_OK;
    case NS_THEME_SCROLLBAR_TRACK_HORIZONTAL:
      aPart = SP_TRACKSTARTHOR;
      aState = TS_NORMAL;
      return NS_OK;
    case NS_THEME_SCROLLBAR_TRACK_VERTICAL:
      aPart = SP_TRACKSTARTVERT;
      aState = TS_NORMAL;
      return NS_OK;
(I defaulted to horizontal because the winstripe theme does.)

Comment 14

[Markus Stange [:mstange]]

Yeah, that looks simpler.

Comment 15

[[e-mail]]

Comment on attachment 439022 [details] [diff] [review]
patch (untested)

This makes things much better although the scrollbar is still too wide for some reason.

Comment 16

[u279076]

Does this still reproduce?

Comment 17

[Wayne Mery (:wsmwk)]

(In reply to Anthony Hughes (:ashughes) [GFX][QA][Mentor] from comment #16)
> Does this still reproduce?

looks like Ken is no longer around

Comment 18

[u279076]

I'm unable to reproduce this locally so unless someone speaks up indicating this is still reproducible it's going to get closed.

Comment 19

[KLB]

Sorry, I didn't see the original requests for info. I'm sure this bug is obsolete, especially since the ability to completely skin themes is being depreciated.  I don't know for sure as I stopped maintaining my theme a couple years ago.

I'll close this bug as resolved, works for me.