Closed Bug 113100 Opened 23 years ago Closed 23 years ago

Stretched icons in link toolbar and personal toolbar folder

Categories

(SeaMonkey :: Themes, defect)

Product:
SeaMonkey
Component:
Themes
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: neutropenia, Assigned: bugs)

References

Details

(Keywords: regression)

Attachments

(7 files, 2 obsolete files)

Screenshot showing stretched icons.
75.81 KB, image/jpeg
Details
another screenshot for comparison
19.21 KB, image/jpeg
Details
one-liner patch to toolbarbutton.css
527 bytes, patch
hwaara
: review+
hewitt
: superreview+
Details | Diff | Splinter Review
Screenshot of the bad link toolbar and dropdown menu icons in build 2001121603
22.83 KB, image/jpeg
Details
patch to bookmarks.css
891 bytes, patch
Details | Diff | Splinter Review
new patch to bookmarks.css
828 bytes, patch
andreww
: review+
Details | Diff | Splinter Review
Corrected corrected icons
4.36 KB, application/zip
bzbarsky
: review+
hewitt
: superreview+
Details 
Win98SE, moz trunk build, Icons seems stretched in the link toolbar and personal
toolbar folder, though other stretched icons have been fixed.
May be related : 
bug 111372
bug 108567
meta bug 111594

Screenshot coming...
See the ugly icons in the link toolbar and the personal toolbar (the latter
being more visible the the betanews bookmark that has a bookmark scheduling
icon. The icons in the tabs are fine though.
Caused by fix for bug bug 108809
Added comment in bug 113027
Icons on personal toolbar look too narrow btw - not stretched.
Component: skinability
Assignee: mpt → ben
Status: UNCONFIRMED → NEW
Component: User Interface Design → Skinability
Ever confirmed: true
OS: Windows 98 → All
QA Contact: zach → pmac
Modern theme only (classic is ok)
Site icons are 16 high by 14 wide (easy to tell from slashdot icon with its
white background.)
*** Bug 113479 has been marked as a duplicate of this bug. ***
Keywords: regression
Attached patch Patch (obsolete) — — Splinter Review 
This patch fixes both the link toolbar bug and the personal toolbar bug.

It removes the hardcoded width/height:16px but keeps the max-width height; for
some reason modern doesn't like it.
rjc and hewitt, wanna review?
Comment on attachment 60386 [details] [diff] [review]
Patch

testing a better solution. hold on.
Attachment #60386 - Attachment is obsolete: true
After grepping in circles for a couple of hours, I've found the real culprit
for this bug - the css rule which creates space between the toolbar icon and
the text. It was using padding-right instead of margin-right, and encroaching
on the icon's space by exactly 2px. This is actually correct behaviour, so the
fix is to change it to use margin-.
Comment on attachment 60391 [details] [diff] [review]
one-liner patch to toolbarbutton.css

I was *just* about to add a patch that accounted for the extra margin, but
cvs.mozilla.org is horked. :)

r=hwaara (but let rjc look at this too before you checkin)
Attachment #60391 - Flags: review+
Comment on attachment 60391 [details] [diff] [review]
one-liner patch to toolbarbutton.css

sr=hewitt
Attachment #60391 - Flags: superreview+
checked in, thanks for the patch chris!
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
I can see in LXR this has been checked in but I still see the stretched icons in
a fresh CVS build pulled two hours after the check-in (after 1700 PST). Modern
theme, WindowsME

Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
It is fixed for me using 2001-12-05-04, Windows 98.
It's fixed in my CVS tree, and on the Mac OSX build 2001-12-05-08.

Jacek, can you check that your copy of
mozilla/themes/modern/global/toolbarbutton.css has the merged changes. Then
delete your chrome.jar before running make in mozilla/themes. Or try building in
a brand new source tree.

If is still not fixed after that, then keep this bug open; otherwise, this is
resolved fixed.
<spam>
Sorry, that say 'delete your modern.jar'
</spam>
It's fixed on linux too.
oops..the link toolbar arrows are still bad
My mozilla/themes/modern/global/toolbarbutton.css has been updated in the build
which still has the problem. I've deleted modern.jar and rebuild. The icons are
still streched in link toolbars like in attachement 60054 as well as in dropdown
menus.

I also installed Win32 installer build 2001120603. The same icons are stretched
here as well. I'll attach a screenshot.
Also deleting XUL.mfl, deleting component.reg or installing in a fresh directory
does not help. The icons cheerfully keep on being streched.
Jacek: menu icons (checkmarks and bullets) being stretched is actually bug
113027 which isn't fixed yet.

By the way, the Home icon in personal toolbar is still too narrow (build
2001-12-06-08 on Linux).
Shouldn't fixing this be kind of straight-forward adding two css lines to the
right file, ala
  width: 7px;
  height: 7px;
or something?
I don't really CSS but this one looks kind of trivial, looking at
/themes/modern/navigator/linkToolbar.css ..
Maybe the problem with  link toolbar icons (seems the only problem left in this
bug) should be spun off as another bug which would allow this one to be closed.
Okay, the home button is still too narrow, and the "site navigation bar" icons 
are stretched. I never saw this before because I don't use this toolbar. I'll 
take a look...
Weird...

Looking at mozilla/themes/modern/communicator/bookmarks/bookmarks.css, all
'bookmark-item's have a width of 16x16px. This is okay for favicons, but not
for the home button (27x17px - very weird)and not for the site navigation bar
(7x7px). It's probably best to _not_ specify a size here, and let the image
scale itself.

This patch removes the 16x16px hard-coded size, and so fixes all the toolbar
buttons. I think it also makes my previous patch obsolete, but whatever...  :o)
Comment on attachment 60745 [details] [diff] [review]
patch to bookmarks.css

you dont need to keep the height and width there as a comment, just remove the
rules alltogether. and the "see bug 113100 is not valid css since it's not in a
comment..
If you remove those 2 lines you added I'll r= it...
Attachment #60745 - Flags: needs-work+
moving component.. this is not "skinnability" - the ability to switch and 
apply  skins - it's  themes - the actual skin itself.
Component: Skinability → Themes
>you dont need to keep the height and width there as a comment,
>just remove the rules alltogether.
Ok.

>and the "see bug 113100 is not valid css since it's not in a
>comment..
Oops... It still works, as mozilla just skips on to the next rule when it finds
a syntax error. Fixed now.
Attachment #60821 - Flags: review+
Comment on attachment 60821 [details] [diff] [review]
new patch to bookmarks.css

r=andreww
Sorry, but any item that can be stamped with a favicon MUST be forced to be 
16x16.  Don't change that.  Allow Ben to come up with other CSS rules to fix 
problems.

*** Bug 114977 has been marked as a duplicate of this bug. ***
I don't know if this is related, but in bug 114977 (now marked as a duplicate of
this) I also reported that the "First" button in the link toolbar of the classic
skin was also messed up... not stretched, but most of it was blank except for a
few dots at the top of it.  I attached a screenshot to that bug.
*** Bug 115418 has been marked as a duplicate of this bug. ***
Dan, that icon has been that way since the link toolbar landing.. the behavior
of that has not changed at all by favicons changes.
Is somebody going to fix it, then?  Does a new bug need to be opened for it?
Might want to ask Hewitt about this - I think he has / had a similar bug.

Andrew, the only one that comes to mind is bug 113027?  Should this be dupe of
that one?
The home button width seems to be fixed now (using build 2001-12-19-08 on
linux), although it breaks again if you dynamically switch between Modern and
Classic themes.
Link toolbar icons are now the correct size in classic, but still stretched in
modern
Build 2001121909 WinME
The one messed up button in the Classic link toolbar that I mentioned earlier
seems to be fixed.  Lots of other skin/theme problems were fixed as of
yesterday's builds.  However, the modern theme link toolbar buttons are still
stretched.
icons in link toolbar still stretched as of 12/28 for modern.  Additionally, for
modern the grayed out versions of Top, Up, First, Previous, Next, and Last are
grayed out versions of the default bookmark icon instead of grayed out versions
of arrows(stretched or otherwise).
I'm not sure how related this is, but recently (since 20011218, IIRC) something
big has happened to the CSS for XUL components... i.e. it's gone. Basically, I
just downloaded 2001123008, and *all* my custom themes (including the one I
wrote) now have toolbar icons stretched all over the place, and some buttons
seem to be missing the -moz-box-orient, as well as the borders on :hover (among
other things).

It seems to be because CSS has been moved from the global XUL files to the
individual theme CSS files, breaking *all* custom themes. Is this because of
this bug, or some other bug I can't find?
Travis (Comment #43) -
Grayed icons reported in defect 116259.
all the css and much of the xbl has changed radically due to Joe Hewitt's 
skin clean up of modern and classic.

Stuart, Tim, would you deal with this one? The icons just need to be edited to 
be 16x16 with more space around the image....
Blocks: 117955
Attached file Corrected icons for modern theme (obsolete) — 
The attachment contains new versions of the link toolbar icons that are 16x16.
When I add these files to modern.jar, the link toolbar arrows are no longer
stretched (they were formerly 11x11). This may or may not fix the underlying
problem, but it does fix this bug. Please implement this and close this bug, it
is incredibly annoying (to me, at least) to be reading bugzilla with a bug
staring me in the face. If not, then I'll just keep patching my local copy ;).
r= and sr= me! easy to get in for 0.9.8!!
It appears in my zealousness I managed to send the jar that has the icons
WITHOUT transparent backgrounds. whoops.
Attachment #65382 - Attachment is obsolete: true
Attachment #65388 - Attachment mime type: application/octet-stream → application/zip
Comment on attachment 65388 [details]
Corrected corrected icons

r=bzbarsky.  Looks good!
Attachment #65388 - Flags: review+
Karl, are you planning on requesting sr= and a= on this and chasing it to
checkin for 0.9.8? (Just checking, because I don't want everyone to think
someone else is doing it)
Adding to linktoolbar metabug
Blocks: 103053
I sent mail asking for sr at the same time as I marked r=....
cool :)
I am NOT pursuing r=, sr=, or a=. I think it'd be a nice thing to have in 0.9.8,
but am currently busy with school and not willing to commit the time.
*** Bug 121113 has been marked as a duplicate of this bug. ***
Comment on attachment 65388 [details]
Corrected corrected icons

sr=hewitt
Attachment #65388 - Flags: superreview+
a=brendan@mozilla.org for 0.9.8 checkin of the fixed icon images.

/be
Blocks: 115520
Keywords: mozilla0.9.8+
Checked in. I think that was the last of it, so marking fixed.  Please reopen if
there are other cases that got missed....
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Yes, finally fixed :). I dunno Why this bug, really got on my nerves!
I don't think this one's really fixed. When looking at the disabled icons, they
all have some white space on the upper/right side, as if they've been moved one
pixel left and/or down. Didn't find any bug on this, so I think this one should
be reopened.
Seeing this with 2002012408.
See bug 117955
Verified
Status: RESOLVED → VERIFIED
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: