Closed Bug 323686 Opened 17 years ago Closed 17 years ago

Toolbar.png needs to be cropped

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set
trivial

Tracking

()

VERIFIED FIXED
Firefox 2 alpha1

People

(Reporter: Gavin, Assigned: philip.puryear)

References

Details

(Keywords: verified1.8.1)

Attachments

(4 files, 2 obsolete files)

In bug 306453, the toolbar item for "Read Mail" was removed, so now Toolbar.png in both winstripe and pinstripe contain an unused icon. It should just be cropped, removing the mail icon. No CSS changes are needed since the icon is the left-most icon in the set.
Attached image Toolbar.png (pinstripe) (obsolete) —
Something like this?

(I can't assign bugs to myself, otherwise I'd do so.)
Attachment #208700 - Flags: review?(gavin.sharp)
Attached image Toolbar.png (winstripe) (obsolete) —
Attachment #208701 - Flags: review?(gavin.sharp)
Comment on attachment 208700 [details]
Toolbar.png (pinstripe)

Yes, that's the idea, except that the pinstripe image should be 448 pixels wide (32px per "button"), and the winstripe one should be 336 pixels wide (24 px per "button"). I also noticed that the Toolbar-small.png images need to be changed as well. I also wonder if there are any special considerations to be made to ensure no loss of quality/transparency/etc when modifying the images (as noted, the editor I had used messed the icons up considerably).
Attachment #208700 - Flags: review?(gavin.sharp) → review-
Attachment #208701 - Flags: review?(gavin.sharp) → review-
When done in Photoshop, transparency shouldn't be an issue, but bicubic resizing of an image will result in a little loss of quality. If that's not an issue, I could just resize the entire image, but if it is I suppose it could be re-pieced together from scratch. Are all those images available somewhere in a higher resolution?
Why do you need to resize the image? If just cropping it at those dimensions cuts off one of the images then something funky is going on :). I'm assuming all the images are of a fixed width... if they're not, this bug is a little harder to fix, since you'd need to check all the CSS-specified coordinates and see what area each button is using, but I doubt that is the case.

And thanks for taking a look at this!
Oh! At first I thought you meant that I needed to somehow reconstruct the entire image from scratch, rather than that mine were just a few pixels off. I'll attach correct ones shortly.
Attachment #208700 - Attachment is obsolete: true
Attachment #208701 - Attachment is obsolete: true
Attachment #208705 - Flags: review?(gavin.sharp)
Attachment #208706 - Flags: review?(gavin.sharp)
What are the required dimensions for Toolbar-small.png?
I just took the current image widths and divided by the current number of icons: so 240/15=16 and 360/15=24 for Winstripe and Pinstripe, respectively.
Attachment #208707 - Flags: review?(gavin.sharp)
Attachment #208708 - Flags: review?(gavin.sharp)
Thanks Philip, I will take a look at these and test tomorrow.
Attachment #208708 - Flags: review?(gavin.sharp) → review+
Attachment #208706 - Flags: review?(gavin.sharp) → review+
Attachment #208707 - Flags: review?(gavin.sharp) → review+
Attachment #208705 - Flags: review?(gavin.sharp) → review+
I tested these on Windows, Jesse tested on Mac, eveything looks good. I will mail the theme author to double-check that this is OK before landing this. Thanks again Philip!
Looks good. Thanks for letting me know Gavin!
Assignee: nobody → philippuryear
Now that the patch for bug 306453 has been approved for 1.8, should I seek similar approval for these images so they can land on the branch along with that?

Also, I noticed that this was checked in yesterday. Can I mark this FIXED yet?
Oops, sorry, I forgot to mark the bug fixed when I checked it in. I'll land this on the branch when I land the patch in bug 306453.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Landed this and bug 306453 on the 1.8 branch for Firefox 2.
Keywords: fixed1.8.1
Target Milestone: --- → Firefox 2
Target Milestone: Firefox 2 → Firefox 2 alpha1
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.