477710 - [Modern] no styles for horizontal scrollboxes

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect)

Description

[Karsten Düsterloh]

Modern lacks styles for horizontal scrollboxes which are no .autorepeatbutton, most prominently rules to show left and right arrows. Basically, the classes .scrollbutton-up and .scrollbutton-down are ignored.

Compare <http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/themes/winstripe/global/scrollbox.css> (Winstripe) with <http://mxr.mozilla.org/comm-central/source/suite/themes/modern/global/scrollbox.css>. (Modern)...

This blocks smtabmail.

Comment 1

[Philip Chee]

Attachment: Patch v1.0

I've no idea how to test. The only thing that we have that needs this is tabmail but that hasn't landed in the tree yet. CC Mnyromyr.

Comment 2

[Ian Neal]

I can confirm that this patch gives left and right arrows on tabmail in modern.

Comment 3

[[e-mail]]

Comment on attachment 396984 [details] [diff] [review]
Patch v1.0

># HG changeset patch
># User Philip Chee <[e-mail]>
># Date 1251368193 -28800
># Node ID 2cc3eec1357363a3fb2660a3e850950b022749fd
># Parent  b58c2df96f561d2f3317cd232340edb5cb1350de
>Bug 477710 [Modern] no styles for horizontal scrollboxes
>
>diff --git a/suite/themes/modern/global/scrollbox.css b/suite/themes/modern/global/scrollbox.css
>--- a/suite/themes/modern/global/scrollbox.css
>+++ b/suite/themes/modern/global/scrollbox.css
>@@ -37,37 +37,94 @@
> 
> /* ===== arrowscrollbox.css =============================================
>   == Styles used by the XUL arrowscrollbox and related elements.
>   ======================================================================= */
> 
> @namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");
> 
> /* ::::: auto-repeat button ::::: */
>- 
>-autorepeatbutton {
>+
>+autorepeatbutton,
>+.scrollbutton-up,
>+.scrollbutton-down {
>   -moz-box-align: center;
>   -moz-box-pack: center;
>   margin-top: 1px;
>   margin-bottom: 2px;
>   -moz-margin-start: 1px;
>   -moz-margin-end: 2px;
I don't think these margins make sense for tab scrollbuttons as they're designed to work with the autorepeatbutton hover effect. I'd prefer a separate style block for scrollbuttons.

>-autorepeatbutton:hover {
>+autorepeatbutton:not([disabled="true"]):hover,
>+autorepeatbutton:not([disabled="true"]):hover:active {
A duplicate :hover:active rule makes no sense.

>+.scrollbutton-up > .toolbarbutton-text,
>+.scrollbutton-down > .toolbarbutton-text {
>+  display: none;
>+}
This doesn't make sense here. Put it before all the image rules.

>+/* XXXRatty: For MOZILLA_1_9_2 replace [chromedir="rtl"] with
>+   :-moz-locale-dir(rtl) */
Don't need this comment here; we should have a bug filed for this though.

>+.scrollbutton-up[chromedir="rtl"][orient="horizontal"][disabled="true"] {
Nit: with these rules my preferred order is orient, chromedir, disabled.

Comment 4

[Philip Chee]

Attachment: Patch v1.1

>>-autorepeatbutton {
>>+
>>+autorepeatbutton,
>>+.scrollbutton-up,
>>+.scrollbutton-down {
>>   -moz-box-align: center;
>>   -moz-box-pack: center;
>>   margin-top: 1px;
>>   margin-bottom: 2px;
>>   -moz-margin-start: 1px;
>>   -moz-margin-end: 2px;
> I don't think these margins make sense for tab scrollbuttons as they're
> designed to work with the autorepeatbutton hover effect. I'd prefer a separate
> style block for scrollbuttons.

Fixed.

>>-autorepeatbutton:hover {
>>+autorepeatbutton:not([disabled="true"]):hover,
>>+autorepeatbutton:not([disabled="true"]):hover:active {
> A duplicate :hover:active rule makes no sense.

Fixed. (too much copy/paste from *stripe)

>>+.scrollbutton-up > .toolbarbutton-text,
>>+.scrollbutton-down > .toolbarbutton-text {
>>+  display: none;
>>+}
> This doesn't make sense here. Put it before all the image rules.

Fixed. (too much copy/paste from *stripe)

>>+/* XXXRatty: For MOZILLA_1_9_2 replace [chromedir="rtl"] with
>>+   :-moz-locale-dir(rtl) */
> Don't need this comment here;

Fixed. (too much copy/paste from *stripe)

>>+.scrollbutton-up[chromedir="rtl"][orient="horizontal"][disabled="true"] {
> Nit: with these rules my preferred order is orient, chromedir, disabled.

Fixed.

Comment 5

[Philip Chee]

Oops Now in code freeze.

Comment 6

[Philip Chee]

Comment on attachment 398105 [details] [diff] [review]
Patch v1.1

Needed for tabmail scrollbox.

Comment 7

[Robert Kaiser]

Not blocking on a minor Modern glitch, but the patch has approval anyway, so it will be fixed and we don't really need to consider if it would push out a release, right? ;-)

Comment 8

[Stefan [:stefanh]]

http://hg.mozilla.org/comm-central/rev/f0d17245b3cb

Comment 9

[Robert Kaiser]

Adding fixed-seamonkey2.0 keyword so the queries for approved but not fixed bugs don't catch that bug.