533348 - Correct changeset 0174b636d5ee (fix broken Modern rules in alert.css/accountCentral.css/pageInfo.css) (new mail notification/download complete/update "toaster" tray area popup text and Page Info Feeds Tab items missing margin/padding)

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect)

Description

[Jens Hatlak (:InvisibleSmiley)]

The file global/alerts/alert.css is responsible for the styling of new mail notifications/download complete/update "toaster" tray area popups. Classic and Modern are mostly the same regarding this file but there is one notable difference:

Modern has:

.alertTextBox {
  margin-top: 4px;
  margin: 0 6px;
}

Classic has:

.alertBox[orient="horizontal"] > .alertTextBox {
  -moz-padding-end: 10px;
  padding-top: 5px;
}

.alertBox[orient="vertical"] > .alertTextBox {
  -moz-padding-start: 5px;
  -moz-padding-end: 5px;
  margin-bottom: 8px;
  -moz-box-align: center; /* also hard-coded in alert.js, see bug 311557 */
}

Don't ask me why but the margin-top doesn't work (anymore?). I'll attach screen shots, a little script to trigger the alerts, and a patch that replaces Modern's rule by Classic's (currently applies cleanly to both trunk and branch).

Comment 1

[Jens Hatlak (:InvisibleSmiley)]

Attachment: apply Classic rules to Modern

Patch applies to both trunk and branch.

Comment 2

[Jens Hatlak (:InvisibleSmiley)]

Attachment: faked Modern download alert

Comment 3

[Jens Hatlak (:InvisibleSmiley)]

Attachment: faked Modern mail alert

Comment 4

[Jens Hatlak (:InvisibleSmiley)]

Attachment: faked Classic download alert

Comment 5

[Jens Hatlak (:InvisibleSmiley)]

Attachment: faked Classic mail alert

Comment 6

[Jens Hatlak (:InvisibleSmiley)]

Attachment: example toaster trigger script

Comment 7

[[e-mail]]

(In reply to comment #0)
> .alertTextBox {
>   margin-top: 4px;
>   margin: 0 6px;
> }

> Don't ask me why but the margin-top doesn't work (anymore?).
Oops. It used to have separate top, right and left margins. They weren't properly combined in bug 474807 :-(

Comment 8

[Jens Hatlak (:InvisibleSmiley)]

FWIW if possible I'd like to have at least the rather striking visual glitch described here corrected on the 2.0 branch. On trunk we might have to fix all rules that were combined incorrectly through the checkin for bug 474807 but that should probably be done in a new bug then, for all affected parties.

Comment 9

[Jens Hatlak (:InvisibleSmiley)]

Attachment: correct changeset 0174b636d5ee [Checkin: comment 11]

This simply fixes the "definite issues" from bug 474807 comment 21 without changing the actual approach (i.e. I left out the .alertTextBox -> alertBox[orient="horizontal|vertical"] > .alertTextBox change).

Notes:
* alert.css: introduces margin-bottom:0 compared to before changeset 0174b636d5ee but that's OK since no other rule sets margin-bottom here (I checked)
* accountCentral.css: the re-introduced padding-left:10px has no visual effect since the .acctCentralRow rule already indents the whole area using -moz-margin-start:10px. If there's a desire to drop the padding-left due to that, IMHO the rule should at least be adapted to read padding: 10px 0;

Comment 10

[[e-mail]]

Comment on attachment 417506 [details] [diff] [review]
correct changeset 0174b636d5ee [Checkin: comment 11]

I don't see why this can't go in 2.0.2, given that it's a medium-reward low-risk regression fix.

Comment 11

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 417506 [details] [diff] [review]
correct changeset 0174b636d5ee [Checkin: comment 11]

http://hg.mozilla.org/comm-central/rev/111318ebf2da
http://hg.mozilla.org/releases/comm-1.9.1/rev/27bc34d4c18a