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)

RESOLVED FIXED in seamonkey2.1a1

Status

SeaMonkey
Themes
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)

Tracking

({fixed-seamonkey2.0.3})

Trunk
seamonkey2.1a1
fixed-seamonkey2.0.3

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 1 obsolete attachment)

(Assignee)

Description

8 years ago
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).
(Assignee)

Comment 1

8 years ago
Created attachment 416463 [details] [diff] [review]
apply Classic rules to Modern 

Patch applies to both trunk and branch.
Assignee: nobody → jh
Status: NEW → ASSIGNED
Attachment #416463 - Flags: superreview?(neil)
Attachment #416463 - Flags: review?(neil)
Attachment #416463 - Flags: approval-seamonkey2.0.2?
(Assignee)

Comment 2

8 years ago
Created attachment 416464 [details]
faked Modern download alert
(Assignee)

Comment 3

8 years ago
Created attachment 416465 [details]
faked Modern mail alert
(Assignee)

Comment 4

8 years ago
Created attachment 416466 [details]
faked Classic download alert
(Assignee)

Comment 5

8 years ago
Created attachment 416467 [details]
faked Classic mail alert
(Assignee)

Comment 6

8 years ago
Created attachment 416468 [details]
example toaster trigger script
(Assignee)

Updated

8 years ago
Attachment #416468 - Attachment mime type: application/octet-stream → text/plain

Comment 7

8 years ago
(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 :-(
(Assignee)

Comment 8

8 years ago
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.
(Assignee)

Comment 9

8 years ago
Created attachment 417506 [details] [diff] [review]
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;
Attachment #416463 - Attachment is obsolete: true
Attachment #417506 - Flags: review?(neil)
Attachment #416463 - Flags: superreview?(neil)
Attachment #416463 - Flags: review?(neil)
Attachment #416463 - Flags: approval-seamonkey2.0.2?
(Assignee)

Updated

8 years ago
Summary: Modern misses changes to global/alerts/alert.css (new mail notification/download complete/update "toaster" tray area popup text is missing top and right margins) → 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)
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.
Attachment #417506 - Flags: review?(neil)
Attachment #417506 - Flags: review+
Attachment #417506 - Flags: approval-seamonkey2.0.2+
(Assignee)

Comment 11

8 years ago
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
Attachment #417506 - Attachment description: correct changeset 0174b636d5ee → correct changeset 0174b636d5ee [Checkin: comment 11]
(Assignee)

Updated

8 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: fixed-seamonkey2.0.2
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1a1
You need to log in before you can comment on or make changes to this bug.