Closed Bug 257480 Opened 20 years ago Closed 20 years ago

theme style and icon cleanup - spit & polish

Categories

(Firefox :: Toolbars and Customization, defect)

1.0 Branch
x86
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: asa, Assigned: bugs)

Details

(Whiteboard: See summary, attachments and comment 2, 5, 6 and 7)

Attachments

(11 files, 6 obsolete files)

4.20 KB, image/png
Details
66.46 KB, image/png
Details
10.16 KB, image/png
Details
592 bytes, patch
Details | Diff | Splinter Review
830 bytes, patch
Details | Diff | Splinter Review
35.80 KB, image/png
Details
18.16 KB, image/png
Details
14.64 KB, image/png
Details
22.77 KB, image/png
Details
1.64 KB, patch
kevin
: review+
Details | Diff | Splinter Review
19.67 KB, image/png
Details
There are some minor issues with the Firefox default theme that we should clean
up in time for 1.0. Rather than file bugs on each of these, I'll just start
noting the various issues here.

If you're adding new problems, please attach screenshots and be as descriptive
as possible. This is not the place to criticize the design of the icons, just
the implementation. 

Issues: 

Reload button has a stray pixel to the right of the top arrow. 
Bookmark icon seems to have a couple of extra pixels in in a horizontal line
just above the base and to the right of the book's binding.
The mail envelope's dropdown arrow is spaced differently than the back and
forward icons'.
The paste clipboard icon has stray white pixels to the right of it. 


more issues as I find them :)
Attached image main toolbar showing icon issues (obsolete) —
I guess these fall into this category:
bug 252718 text on right of Find Toolbar misaligned by 1 pixel
bug 256048 disabled version of clean up button icon is missing
bug 257379 close tab button gives visual feedback on a larger area than the button

Asa - I think for some of the things you mention, it might help to make clear
whether you're talking about large or small versions of the button icons?
sorry, yeah. those are large icon versions. I'll be more specific going forward.
The screenshots should make it obvious though. 

Also, I know that some of these issues may be with nsITheme. I don't think we'll
be making major changes to that before ship so if there's anything we can do to
make it better in the css and images, that's my hope. 
Our toolbars have shadow borders at or near #ACA899 and for Luna (our default
look) that should be closer to #D8D2BD	Also, the height of the splitter "icon"
isn't tall enough.
Our menus are also a bit funky when it comes to colors, borders, and spacing.
See bug 253661 for a good description of the issues.
Another small theme icon bug brought up in my blog comments:

If you click on a toolbar button (large/small icons), the icon moves to the
right by one pixel, but not down. Standard toolbar buttons in Windows (e.g. IE)
moves down *and* to the right by one pixel. 
Whiteboard: See summary, attachments and comment 2, 5, 6 and 7
One more bug:
There are two 'History' menu items.
1. View > Sidebar > History
2. Go > History
Both doesn't act the same way, as the former is a [type="checkbox"] menu item,
the latter is a normal menu item. IMO, they should be the same.

PS: I don't feel like posting 'Firefox Dust 3' on my blog, so I better post bugs
here.
comment 8 is fixed in today's builds.
Bug 253738 Stephen Horlander's tracker (although that's probably more for major
changes and updates than "spit and polish")

Many of these already have patches ready to go or working userChrome fixes not
yet in patch form.

Bug 177507 no bookmarks toolbar chevron in text mode
Bug 232550 button label widths
Bug 235300 no bookmark icon in palette (as mentioned in cheeaun's "dust 2" -
similar solution could be used to help distinguish and identify spacers and springs)
Bug 172254 customize palette border
Bug 227745 search icon dropmarker
Bug 235277 make go button look like other toolbar buttons
Bug 226274 dropmarker hover states (i think this is pretty bad, but the problem
probably runs a little deeper than a simple theme fix)
Bug 215839/bug 217293 text label vertical alignment (another potentially deeper
problem in nsITheme
Bug 246186 tab bar 1px taller with more than one tab

-The full-screen mode buttons don't stick to the right of the screen if the
toolbar isn't full.  That's a simple fix two-line fix.  Should we just pin
patches to this bug?  Ah, Bug 248330 might cover this, though the current patch
doesn't fix the right-stickiness.
I'm not sure if this can be fixed at the theme level, but buttons currently
have a small 2px horizontal "gap" between them. This means there is a dead spot
between buttons that provides no value and probably hurts usability some. 

The space is highlighted with the green circle. The image below is IE for
comparison.
Since this is about theme, might I point your attention to bug 253367. This is a
trivial UI bug that, if fixed, would require a minor theme change as well. This
is because current theme actually "takes advantage" of that buglet.

The minor theme change is already in patch to that bug - so unless you want to
WONTFIX that bug 253367 (which I don't recommend, since I'm the reporter), you
might want to review+ that patch with the other theme polish patch(es).
When you try dragging the bookmarks on the bookmarks toolbar, there are no
drag-over lines/borders, only happens on systems that reads -moz-appearance, I
think. I tried fixing it with this:


toolbarbutton.bookmark-item[dragover-left="true"],
toolbarbutton.bookmark-item[dragover-right="true"],
toolbarbutton.bookmark-item[dragover-top="true"],
toolbarbutton.bookmark-item[dragover-bottom="true"] {
-moz-appearance: none !important;
}

but not a perfect solution, yet.
(In reply to comment #11)
> I'm not sure if this can be fixed at the theme level, but buttons currently
> have a small 2px horizontal "gap" between them. This means there is a dead
> spot between buttons that provides no value and probably hurts usability some. 
> 
> The space is highlighted with the green circle. The image below is IE for
> comparison.

This only appears to be an issue on XP Luna.
(In reply to comment #11)
(In reply to comment #14)
> This only appears to be an issue on XP Luna.

Nope.  I got it on Win2000 with the latest branch build.  It's caused by 2px of
right margin on browser toolbarbuttons
(browser/browser.css/.toolbarbutton-1,.toolbarbutton-menubutton-button).  I
think that's been a recent -stripe theme change, but i don't understand why it's
there.
Stephen, do you know?

(In reply to comment #13)
> No drag-over borders on bookmarks toolbar
Yeah, looks to be a problem on platforms with nsITheme-enabled themes - looks
fine on WinClassic.  Very similar to Bug 226274 in regards to dynamic-state
borders and such on native-styled elements.
a few other quick things...

The style rules for the Print Preview/Page Setup page orientation icons point to
non-existent images.  Images live in the global/icons dir.

The expander button on the Add Bookmarks dialog looks kinda tacky.  Might
benefit from a little padding or something to make it a bit larger.

Should not the View > Sidebar > menuitems be radios instead of checkmarks?  Only
one can be open at any time.
(In reply to comment #4)
> our splitter is the wrong height and the toolbar borders are too dark

On WinClassic (Win2k), the colors look the same - 50%gray and white.  But i
can't test Luna.
As for the height - i think it's correct.  If you check a few different apps,
most obviously MS Office, you'll notice there's two different "splitters."  One
splitter that separates buttons within a toolbar, and another full-height
splitter that separates different toolbar groups.  This is pretty consistent in
the few apps i've checked so far.  Mozilla toolbars aren't exactly analogous,
because you add loose buttons to hard horizontal rows.  In native apps, buttons
are grouped into "toolbars" (Standard Buttons, Address Bar, Links, etc.) that
can then be placed into a flexible space.  You can also often drag, float, and
dock those toolbar groups, but that's a whole 'nother issue.
This removes the dead space between toolbar buttons.  And though i can't figure
it out, there may yet be some reason it was added in the first place.
corrects the stylesheet to point to the images in the icons directory
corrects appearance of the Mail button dropmarker
Attached image screenshot: icon areas
The icons aren't always centered in the image areas.  Sometimes the icons are
smaller than 32x32 or 24x24, but for various reasons, aren't centered in the
image area.  A few of the Options icons, for example, are way off, relatively
speaking.  The generic bookmarks icon, Reload, large Downloads, and New Window
are some that stand out to me.
Attached image missing dropmarkers in Text mode (obsolete) —
There are no dropmakers when switched to 'Text' mode. I supposed they *should*
be visible.
Attached patch patch: dropmarkers in text mode (obsolete) — Splinter Review
(In reply to comment #22)
> missing dropmarkers in Text mode

Not sure if this is intended behavior, but this one line patch does it.
I think, this happens on systems reading -moz-appearance too.
(In reply to comment #21)
> Created an attachment (id=158761)
> screenshot: icon areas
> 
> The icons aren't always centered in the image areas.  Sometimes the icons are
> smaller than 32x32 or 24x24, but for various reasons, aren't centered in the
> image area.  A few of the Options icons, for example, are way off, relatively
> speaking.  The generic bookmarks icon, Reload, large Downloads, and New Window
> are some that stand out to me.

The reason for this is because,  for the most part, the icon content is 22x22
using the remaining space for the drop shadow. So the icons themselves get moved
up and left in the canvas so the shadow has room. I guess the most obvious fix
would be to add one or two extra pixels padding to the left side of the icon.
Although the shadow is technically part of the icon, so it is more of a question
of balancing the visual illusion of being centered. 

The Preferences icons are way off, but they are being redone so it is a 
non-issue at this point.
(In reply to comment #26)
> ...I guess the most obvious fix
> would be to add one or two extra pixels padding 
> to the left side of the icon. Although the shadow 
> is technically part of the icon, so it is more of 
> a question of balancing the visual illusion of being centered.

When i made icons for my theme, i left about 3 pixels of padding around the
outside of the image into which the actual icon part of the image could not go.
 Then the shadows were added afterwards and flowed out into the padding, which
kept the icons visually centered, even though not technically centered
by-the-pixels.

The quick dirty hack would be to just define shifted -moz-image-region values to
compensate until the images can be fixed.
Fixing stray pixels. New Copy/Paste/Cut icons coming but I removed the white
pixels anyway. Softened the pixels on the bottom of the Bookmarks icon, it is
actually supposed to be that way because it is *slightly* tilted.
If you see very carefully, the image is actually stretched (slightly), compared
to the original image file.
Re: comment #29. The URL bar padlock icon should be 16x16, but is displaying at
16x20 (at least in Windows Classic or Luna without any font size changes).
Increase the font size or otherwise increase the height of the URL bar, and the
icon grows accordingly. I'm thinking the icon should stay the same size
regardless, or perhaps scaled proportionally (though this would be harder to
implement).

The padlock icon in the status bar is the correct size, however, even though the
style rule is pretty much similar. For comparison:

URL bar icon:
#urlbar[level="high"] > .autocomplete-textbox-container > .info-icon {
  list-style-image: url("chrome://browser/skin/Secure.png");
}

Status bar icon:
#security-button[level="high"] {
  list-style-image: url("chrome://browser/skin/Secure.png");
  display: -moz-box;
}

Note that adding "display: -moz-box;" to the URL bar icon rule does nothing. I'm
thinking it's inheriting height elsewhere, but I can't quite figure out where.
Note that adding width or height properties won't work either, and using
min-height or max-height has some interesting side effects. (Try adding the
"max-height: 16px;" to the property via the DOM Inspector, and you'll see what I
mean.)
The problem was the inner textbox container with the security icon, text area,
and site icon was inheriting "-moz-box-stretch", which makes its children
stretch to fill its height.  The site icon was actually vulnerable to this as
well, but it has a margin around it which made its dimensions just fit into the
text container at normal sizes.  I fixed it all the way upstream in
global/autocomplete.css.

(In reply to comment #30)
> ...or perhaps scaled proportionally
> (though this would be harder to implement).

Proportional icon scaling is possible, if the width and height were defined in
ex units.  Then they should scale with the system font size.  Though scaling to
the size of the textbox would certainly be trickier.

> and using min-height or max-height 
> has some interesting side effects.

In my experience, making live changes to some elements with DOMI can have
dangerous results, so don't always trust it.
When hovering over the back/forward button dropmarkers they do not highlight
like the rest of the buttons, is this by design?
Also right-clicking on the back/forward buttons performs the same action as a
left click instead of bringing up the standard toolbar context menu.
(In reply to comment #32)
> When hovering over the back/forward button dropmarkers they do not highlight
> like the rest of the buttons, is this by design?

As far as I know, this only happens in Windows Classic in XP/older versions of
Windows. I agree that it should be more obvious. There's a definite rollover
state using Luna and under GTK2. (I don't have a Mac, so I don't know how it
behaves there.) Anyway, there's already a separate bug for this, bug 216266.

> Also right-clicking on the back/forward buttons performs the same action as a
> left click instead of bringing up the standard toolbar context menu.

Right-clicking on the back and forward buttons should bring up a history menu.
This is by design; this behavior is inherited from the Mozilla suite/Netscape 6
and 7, which in turn was inherited from Netscape 4. IE for Windows has this
behavior as well. I suspect that this is unlikely to change in the near future,
given the feature's entrenchment.
I checked in some spit & polish tonight on the branch

#252718, find toolbar misaligned
#256048, cleanup button in DL window
#177507, show chevron in text mode
#235300, show bookmarks icon in cust. window
#257480, print preview icons
#257480, lock icon stretched in URL bar
#257480, show dropmarkers in text mode
#257480, flex. space icon

Thanks miahz and everyone who submitted patches!
how about bug 262030?
Copies the changes to autocomplete.css and toolbar.css to the respective
gnomestripe file.

Also obsoleting a bunch of screenshots and patches which I believe have been
fixed already. Not sure about the rest.
Attachment #157459 - Attachment is obsolete: true
Attachment #158739 - Attachment is obsolete: true
Attachment #158968 - Attachment is obsolete: true
Attachment #159057 - Attachment is obsolete: true
Attachment #160737 - Attachment is obsolete: true
Attachment #161338 - Attachment is obsolete: true
Comment on attachment 162218 [details] [diff] [review]
gnomestripe port: lock icon stretched in URL bar, flex. space icon [checked in: comment 39]

Kevin, please have a look, preferably before midnight :)
Attachment #162218 - Flags: review?(webmail)
Comment on attachment 162218 [details] [diff] [review]
gnomestripe port: lock icon stretched in URL bar, flex. space icon [checked in: comment 39]

Looks good :) Thanks Steffen!
Attachment #162218 - Flags: review?(webmail) → review+
Version: unspecified → 1.0 Branch
Comment on attachment 162218 [details] [diff] [review]
gnomestripe port: lock icon stretched in URL bar, flex. space icon [checked in: comment 39]

Checked into branch 2004-10-15 12:42.
Attachment #162218 - Attachment description: gnomestripe port: lock icon stretched in URL bar, flex. space icon → gnomestripe port: lock icon stretched in URL bar, flex. space icon [checked in: comment 39]
kevin et al., would it be possible to fix bug 262539 as part of this cleanup
bug? (it's still an issue with 2004101506-0.9+ on Mac OS X 10.3.5.)
OS: Windows XP → All
What do you guys think about a listbox-styled palette in the Customize window,
with an inset border and white background?
Now that version 1.0 is out the door, I'd like to close this bug.

We already have the tracking bug for Winstripe (bug 244691). Some of the issues
in the comments above have not been addressed and they should be spun off into
their own reports so we can track and prioritize them seperately. I prefer this
to the one free-for-all bug report.

What do you think Asa?
Resolving as fixed. Please do not reopen this bug. If your issue isn't fixed on
the trunk, then please file a new bug on it. Thanks. 
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
QA Contact: bugzilla → toolbars
You need to log in before you can comment on or make changes to this bug.