348837 - [Mac] 2.0 theme: bottom of toolbarbutton.bookmark-item images so light that they appear to be distorted

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Wayne Woods]

The new images for the buttons on the bookmarks toolbar sport a very light single row of pixels at the very bottom. This blends into the background colour of the toolbar and makes it look like the buttons are squashed or truncated at the bottom. It's especially noticeable when you add icons to the buttons... 16 px high ones appear to be larger than the button.

This affects both hover and open images, so probably 6 images in total.

Comment 1

[Jay Goldman]

Wayne -

Thanks for the feedback! I don't think I'm seeing what you're seeing. I'm attaching a screenshot of my Bookmarks Window Toolbar - can you verify that it suffers from the same problem? If not, can you screenshot yours and attach (shift-cmd-4 to get the screenshot crosshair, click and drag to capture a region or hit space to toggle between crosshair and window capture modes).

Thanks!

Comment 2

[Jay Goldman]

Attachment: Screenshot of Bookmarks Window Toolbar (no squashing?)

Comment 3

[Mark Mentovai]

Attachment: 1.5 v. 2.0

Not the toolbar of the bookmark manager, the bookmark toolbar in a browser window.

This image also shows another problem: because the hover effect has been lightened, there's now much less contrast between the text and its hover surround, making it difficult to read.

Comment 4

[Jay Goldman]

Attachment: Bookmark hover graphics darkened and edged

This should fix both problems by eliminating the light pixel across the bottom and darkening the whole hover bar.

Comment 5

[Wayne Woods]

They look good to me! Not tempted to try an 18 px version, are you? ;) (to accommodate favicons better, as per bug 348719)

Comment 6

[Jay Goldman]

(In reply to comment #5)

> They look good to me! Not tempted to try an 18 px version, are you? ;) (to
> accommodate favicons better, as per bug 348719)

Posted to 348719 :)

Comment 7

[Mark Mentovai]

Comment on attachment 234178 [details]
Bookmark hover graphics darkened and edged

This fixes this bug (348837), although I did like the effect created by the light-bottom image.  I was kinda hoping we could keep that look and just expand the height of the image by a pixel and make necessary CSS changes to accomodate it.  I'm pretty sure I'm being too picky, though.

I don't think that the new hover state is dark enough to really fix bug 348863, although it's better than it was.

This fixes bug 348865 for hovered bookmarks, but not for opened (clicked) bookmarks.

Comment 8

[Jay Goldman]

(In reply to comment #7)
> I was kinda hoping we could keep that look and just expand the height of the image by a pixel

The image is basically the same, but with the 1 px white bar across the bottom replaced by a gray bar instead. Was it the white line you liked?

> I don't think that the new hover state is dark enough to really fix bug 348863,
> although it's better than it was.

I can read the text really well in my build, but (as previously noted), I'm not a great test for legibility concerns 'cause I have good eyes.

> This fixes bug 348865 for hovered bookmarks, but not for opened (clicked) bookmarks.

Screenshot to follow - not sure I'm still seeing this but maybe I'm looking in the wrong place?

Comment 9

[Jay Goldman]

Attachment: Screenshot of livemark hover/open

This is a screenshot of my local build with the new hover images in - top bar is on hover, bottom bar is opened (the folder was empty so there's no menu off it). Comparing it to Mark's screen shot from https://bugzilla.mozilla.org/show_bug.cgi?id=348865 (see https://bugzilla.mozilla.org/attachment.cgi?id=234022), I'm not seeing the same darker bit behind the dropmark on the folder.

Comment 10

[Mark Mentovai]

Yeah, I thought the white line really made the recessed appearance believable, given a light source to the north.

I absolutely still see bug 348865 for opened livemarks and folders.  I tried today's official build just to be sure, and I see it there too.

A note on the legibility concerns: my eyes are fine (until I take out my contacts!) and I have no trouble making out the white-on-light hover text when viewing an LCD at the optimal angle.  The problem is that it becomes illegible when the angle is even only slightly suboptimal, when I can't perceive any problems reading other higher-contrast text on the screen.  Because the rest of the screen looks fine, and the bookmark text only becomes illegible when it's hovered, there's a tendency to not realize that the viewing angle is suboptimal and adjust it.

Comment 11

[Wayne Woods]

I don't mind some gradation. The problem was just that the bottom line was so light that it wasn't really discernible from the background.

Anyway, thanks for the 18px size, Jay. Should we wait till it's in before checking the icon patch, or do you want me to work something up to submit simultaneously?

Comment 12

[Jay Goldman]

(In reply to comment #11)

> Anyway, thanks for the 18px size, Jay. Should we wait till it's in before
> checking the icon patch, or do you want me to work something up to submit
> simultaneously?

I believe it didn't make it in for b2, though mconnor can confirm. I don't have CVS access, so posting it here for review is as far as I go :)

Comment 13

[Wayne Woods]

(In reply to comment #12)
> I believe it didn't make it in for b2, though mconnor can confirm. I don't have
> CVS access, so posting it here for review is as far as I go :)

I've submitted a patch on bug 348719, but it'll require the new 18px buttons to work right. Are you still going to finish them, Jay? :) Hopefully there's enough time between b2 and rc1 that they'll allow us to submit this. If not, it should go on the trunk (and 1.8 branch after 1.8.1 forks) anyway, so it won't be wasted.

Comment 14

[Jay Goldman]

(In reply to comment #13)
> it'll require the new 18px buttons to work right. Are you still going to 
> finish them, Jay? :) 

Er... thought I had? From Comment #11:

> Anyway, thanks for the 18px size, Jay.

Comment 15

[Wayne Woods]

(In reply to comment #14)
> (In reply to comment #13)
> > it'll require the new 18px buttons to work right. Are you still going to 
> > finish them, Jay? :) 
> Er... thought I had? From Comment #11:

Oh, you submitted the "hover" ones on bug 348719 comment 18, which were good for testing, but not the "open" versions. For my own complete testing, I just copied your hover ones, coloured them blue and renamed them. :)

Please also see bug 348719 comment 19... the new images are quite blunt/flat on the ends.

Thanks :)

Comment 16

[Mike Connor [:mconnor]]

landed these images to fix this and bug 348863.  bug 348865 is halfway fixed now.

Comment 17

[Pam Greene]

Note that the bookmark-*-right images need to be 280px wide to (re-)fix bug 343897.

Comment 18

[Wayne Woods]

(In reply to comment #16)
> landed these images to fix this and bug 348863.  bug 348865 is halfway fixed
> now.

Mike, I presume you landed the earlier (16px-high) images, and not the later (18px-high) ones?

Comment 19

[Pam Greene]

The previous set fixed the hover images, but not the active/open set.

Comment 20

[Pam Greene]

Attachment: Fixed hover and open images

Here are the open images with a slightly darker bottom edge, plus hover images with slightly more rounded top corners to better match the open ones, so they don't change shape so much when clicked.

Comment 21

[Pam Greene]

Attachment: Comparison screenshots

Comment 22

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 238014 [details]
Fixed hover and open images

Thanks, Pam. uir+a=beltzner, watch out for the related microsummaries hover effect bug and the folder/rss icon background not matching bug which seem to always regress when we touch these :)

Comment 23

[Pam Greene]

Landing images on 1.8.1 branch.