Closed Bug 395454 Opened 14 years ago Closed 14 years ago

[Mac] Have one file, scrollbars.css

Categories

(Toolkit :: Themes, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9beta5

People

(Reporter: cbarrett, Assigned: cbarrett)

References

Details

Attachments

(1 file, 1 obsolete file)

Now that we no-longer use or have native scrollbars (bug 377457 bug 379319 bug 370439), we don't need "xulscrollbars.css" and "nativescrollbars.css" files on Mac. We should just have scrollbars.css, like Windows.

Nothing should be using xulscrollbars.css, we can just remove it and rename nativescrollbars.css to scrollbars.css (and remove a mac specific ifdef http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/style/nsLayoutStylesheetCache.cpp&rev=1.12#83 ).
Flags: blocking1.9?
Assignee: nobody → cbarrett
What's the perf win here?  I'm -'ing this.  Making it wanted.  If this is something that can get us perf, we should probably do it.
Flags: wanted1.9+
Flags: blocking1.9?
Flags: blocking1.9-
Attached patch fix v1.0 (obsolete) — Splinter Review
I tested this with Firefox, Seamonkey and Camino on Mac. Worked for me.

Enn, if you're not the right reviewer, let me know who is.
Attachment #296461 - Flags: review?
Comment on attachment 296461 [details] [diff] [review]
fix v1.0

Oops
Attachment #296461 - Flags: review? → review?(enndeakin)
Comment on attachment 296461 [details] [diff] [review]
fix v1.0

Seems OK. Technically, you probably shouldn't change the license on themes/classic/global/mac/xulscrollbars.css

The default is horizontal for scrollbars, perhaps we should reverse these:

+
+slider {
+  -moz-appearance: scrollbartrack-vertical;
+}
+
+slider[orient="horizontal"] {
+  -moz-appearance: scrollbartrack-horizontal;
+}

You should ask Neil Rashbrook for a review as well.
Attachment #296461 - Flags: review?(enndeakin) → review+
(In reply to comment #4)
> (From update of attachment 296461 [details] [diff] [review])
> Seems OK. Technically, you probably shouldn't change the license on
> themes/classic/global/mac/xulscrollbars.css

Ah, that happened because I moved nativescrolllbars.css to xulscrollbars.css and CVS thought I just changed the file (it doesn't know).

Suggestions?

> The default is horizontal for scrollbars, perhaps we should reverse these:

I don't know, if you think so, let's do it :)


> You should ask Neil Rashbrook for a review as well.

Will do, thanks!

Attachment #296461 - Flags: review?(neil)
Comment on attachment 296461 [details] [diff] [review]
fix v1.0

Looks good to me, although
a) I did wonder why you didn't change the internal name back to scrollbars.css
b) instead of "renaming" nativescrollbars.css in the tree you can simply rename it in the jar.mn e.g.
 skin/classic/global/scrollbars.css (nativescrollbars.css)
Attachment #296461 - Flags: review?(neil) → review+
(In reply to comment #6)
> (From update of attachment 296461 [details] [diff] [review])
> Looks good to me, although
> a) I did wonder why you didn't change the internal name back to scrollbars.css
> b) instead of "renaming" nativescrollbars.css in the tree you can simply rename
> it in the jar.mn e.g.
>  skin/classic/global/scrollbars.css (nativescrollbars.css)

I like the sound of both of those. New patch soon.
Attached patch fix v1.1Splinter Review
Attachment #296461 - Attachment is obsolete: true
Attachment #297061 - Flags: review?(neil)
Attachment #297061 - Flags: review?(enndeakin)
Attachment #297061 - Flags: review?(neil) → review+
Attachment #297061 - Flags: review?(enndeakin) → review+
Attachment #297061 - Flags: superreview?(bzbarsky)
Please ask someone else for sr?  I doubt I can get to this before the beta freeze, for one thing.
Attachment #297061 - Flags: superreview?(bzbarsky) → superreview?(roc)
Attachment #297061 - Flags: superreview?(roc) → superreview+
Attachment #297061 - Flags: approval1.9?
Comment on attachment 297061 [details] [diff] [review]
fix v1.1

a1.9+=damons
Attachment #297061 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Checking in extensions/inspector/resources/content/tests/allskin.xul;
/cvsroot/mozilla/extensions/inspector/resources/content/tests/allskin.xul,v  <--  allskin.xul
new revision: 1.7; previous revision: 1.6
done
Checking in layout/style/nsLayoutStylesheetCache.cpp;
/cvsroot/mozilla/layout/style/nsLayoutStylesheetCache.cpp,v  <--  nsLayoutStylesheetCache.cpp
new revision: 1.13; previous revision: 1.12
done
Checking in suite/themes/modern/jar.mn;
/cvsroot/mozilla/suite/themes/modern/jar.mn,v  <--  jar.mn
new revision: 1.185; previous revision: 1.184
done
Checking in themes/classic/jar.mn;
/cvsroot/mozilla/themes/classic/jar.mn,v  <--  jar.mn
new revision: 1.169; previous revision: 1.168
done
Checking in themes/classic/global/mac/nativescrollbars.css;
/cvsroot/mozilla/themes/classic/global/mac/nativescrollbars.css,v  <--  nativescrollbars.css
new revision: 1.12; previous revision: 1.11
done
Checking in toolkit/themes/pinstripe/global/jar.mn;
/cvsroot/mozilla/toolkit/themes/pinstripe/global/jar.mn,v  <--  jar.mn
new revision: 1.40; previous revision: 1.39
done
Checking in toolkit/themes/pinstripe/global/nativescrollbars.css;
/cvsroot/mozilla/toolkit/themes/pinstripe/global/nativescrollbars.css,v  <--  nativescrollbars.css
new revision: 1.10; previous revision: 1.9
done
Checking in toolkit/themes/pmstripe/global/jar.mn;
/cvsroot/mozilla/toolkit/themes/pmstripe/global/jar.mn,v  <--  jar.mn
new revision: 1.8; previous revision: 1.7
done
Checking in toolkit/themes/winstripe/global/jar.mn;
/cvsroot/mozilla/toolkit/themes/winstripe/global/jar.mn,v  <--  jar.mn
new revision: 1.46; previous revision: 1.45
done
Status: NEW → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta5
Depends on: 423367
One undesirable side effect of this change is that while Mac Firefox adopted old nativescrollbars.css as the new scrollbars.css, Windows Firefox adopted the old xulscrollbars.css as the new scrollbars.css.  

The problem is if the old xulscrollbars.css is used as the new scrollbars.css on a cross OS custom theme, Mac Firefox loses its scrollbars.  If the old nativescrollbars.css is used for the new scrollbars.css for a custom theme on Windows XP using the "Windows XP Style" desktop a stray black border appears around the scrollbars due to WinXP Firefox not handling "-moz-appearance: scrollbar" properly(see bug 423780).

Since the old scrollbar style sheets have been "merged", we really need Firefox to be fixed such that the Windows Firefox can use "-moz-appearance: scrollbar" without causing a black border.  This fix would allow Windows Firefox to use the old nativescrollbars.css as the new scrollbars.css as Mac does.  This in turn would make cross OS theme development much easier.
Neil, do we need to do something about this?
Depends on: 423780
Product: Core → SeaMonkey
Product: SeaMonkey → Toolkit
QA Contact: themes → themes
You need to log in before you can comment on or make changes to this bug.