Closed
Bug 505334
Opened 16 years ago
Closed 16 years ago
Clean up netError.css and config.css and remove alert-exclam.gif
Categories
(Camino Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: alqahira, Assigned: alqahira)
References
()
Details
(Whiteboard: [camino-2.0])
Attachments
(3 files)
42.17 KB,
patch
|
phiw2
:
review+
stuart.morgan+bugzilla
:
superreview+
|
Details | Diff | Splinter Review |
9.57 KB,
image/png
|
Details | |
13.98 KB,
text/plain
|
Details |
We've been using a data: URI for the image in netError.css and config.css ever since 2006, but we don't need to; it makes the CSS file messy, and that image has some whitespace built in that really should live in CSS rules, so any time we (may) want to change things, we have to specially tune the image to match.
We should instead fix the background positioning in CSS and remove the data: URI in both files, and land the actual icon file in CVS instead.
In addition, we should cvs remove "alert-exclam.gif" from embed-replacements; that was a stop-gap measure until we got the badged icons we use now, and the file is entirely unused now that we're pulling toolkit CSS (it is ultimately the source icon for the badge, but that's all).
Once bug 479554 is in and netError.css is clear, I'll post a patch.
Assignee | ||
Comment 1•16 years ago
|
||
Apparently the Camino in the new blacklist.png icon is about 2px wider (via 2px on the left) and 1px "taller" (via a pixel on the bottom) than the Camino in Jon's 100x100px icon. In netError.css, this CSS gets us to within 1px of each other on the left, and to within 1px from the old 109x108px warning icon data: URI on the top, according to an intense Pixie session :P
(For the morbidly curious, the "whitespace between the canvas edge and the first pixel" measurements are:
Old warning: top=0px, left=14px (this had 11px of padding)
New warning: top=1px, left= 3px
Blacklist: top=1px, left= 1px
With all the CSS rules, the effective number of pixels between the bottom/left edges of the border and the first image pixel of the old warning icon was top=18px, left=14px; now it's top=19px, left=14px for warning, and top=19px, left=13px for blacklist.)
For all practical purposes, it's close enough unless we want to switch to pixels; most users aren't going to pull out Pixie to compare between 1.6.x and 2.0 versions and between the error page and the blocked site page. We were originally using ems for these measurements, so in addition to the necessary left-edge adjustments needed to compensate for the padding in Jon's 109x108px warning image, I switched the blacklist measurements to ems for consistency.
In config.css, it was simply a s/data:*/chrome://global/skin/icons/warning.png/ since philippe had used the 100x100px version in there to begin with. :)
Attachment #389605 -
Flags: review?(phiw)
Assignee | ||
Comment 2•16 years ago
|
||
This is Jon's 100x100px warning.png; I've simply pngcrushed it to save a few more bytes. It will live in embed-replacements/skin/classic/global/icons
![]() |
||
Comment 3•16 years ago
|
||
Comment on attachment 389605 [details] [diff] [review]
CSS changes, v1
in netError.css
>+ background: url("chrome://global/skin/icons/warning.png") 1.0em 1.6em no-repeat -moz-Field;
> }
nitpick:
1em instead of 1.0em
------------------
I had been thinking about using -moz-background-origin:content for positioning the image, but upon reviewing the stylesheet, that would imply much more rewriting, particularly margin values here and there.
Attachment #389605 -
Flags: review?(phiw) → review+
![]() |
||
Comment 4•16 years ago
|
||
Fwiw, here is a diff doing what I mentioned in comment 3 (using -moz-background-origin:content).
The only difference is that it result in a slightly narrower outer box when ma-width is in action (which is pretty fast - 520px), due to the narrower left-padding. For some reason, I like that :-).
This can be corrected, though: setting the max-width to a higher value (eyeballing: 56em;).
Assignee | ||
Comment 5•16 years ago
|
||
Comment on attachment 389672 [details]
netError-diff
I really prefer the larger bounding box (and I'd also like to limit the scope of this bug); I think we can revisit the subject of -moz-background-origin:content when we rework the layout for about:safebrowsingblocked, though.
Assignee | ||
Updated•16 years ago
|
Attachment #389605 -
Flags: superreview?(stuart.morgan+bugzilla)
Assignee | ||
Comment 6•16 years ago
|
||
Comment on attachment 389605 [details] [diff] [review]
CSS changes, v1
I've fixed the nit locally.
Updated•16 years ago
|
Attachment #389605 -
Flags: superreview?(stuart.morgan+bugzilla) → superreview+
Comment 7•16 years ago
|
||
Comment on attachment 389605 [details] [diff] [review]
CSS changes, v1
sr=smorgan
Assignee | ||
Comment 8•16 years ago
|
||
Checked in on cvs trunk and CAMINO_2_0_BRANCH.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Whiteboard: [camino-2.0]
You need to log in
before you can comment on or make changes to this bug.
Description
•