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)

(deleted), image/png
Details
(deleted), image/png
Details
(deleted), image/png
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), image/png
Details
(deleted), image/png
Details
(deleted), image/png
Details
(deleted), image/png
Details
(deleted), patch
kevin
: review+
Details | Diff | Splinter Review
(deleted), 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) (deleted) —
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.
Attached patch patch: fix Page Setup orientation icons (obsolete) (deleted) — Splinter Review
corrects the stylesheet to point to the images in the icons directory
corrects appearance of the Mail button dropmarker
Attached image screenshot: icon areas (deleted) —
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) (deleted) —
There are no dropmakers when switched to 'Text' mode. I supposed they *should* be visible.
Attached patch patch: dropmarkers in text mode (obsolete) (deleted) — 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.
Attached image Fixed toolbar icon quirks (deleted) —
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.
Attached image stretched security icon on the location field (obsolete) (deleted) —
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.)
Attached patch patch: fixes stretched icons in textboxes (obsolete) (deleted) — Splinter Review
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.

Attachment

General

Created:
Updated:
Size: