If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Stop scaling cut/copy/paste and zoom control icons on Retina.

RESOLVED FIXED in Firefox 29

Status

()

Firefox
Theme
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mconley, Assigned: mconley)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 29
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Australis:P-])

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

4 years ago
Bug 875163 was a stopgap patch way back in the day that was used to control the size of the panel icons.

It looks like part of it is coming back to haunt us - for Retina displays, we're adjusting the width to 20px, which scales the images up.

It doesn't stop there - even after removing that, we have another rule that's scaling those icons to 18px (CSS pixels). 18px is what we use for the other toolbar icons. They're supposed to be 16px for the small menu panel icons.

Scaling is expensive, and jrmuizel thinks this is contributing to our slow customization mode transition.
(Assignee)

Comment 1

4 years ago
Created attachment 8365156 [details] [diff] [review]
Patch v1

This should unblur the icons on Retina, and save us a little bit of painting time to boot.
Assignee: nobody → mconley
Status: NEW → ASSIGNED
Attachment #8365156 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 2

4 years ago
Comment on attachment 8365156 [details] [diff] [review]
Patch v1

On second thought, I think I like that rule in panelUIOverlay.inc.css instead.
Attachment #8365156 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 3

4 years ago
Created attachment 8365160 [details] [diff] [review]
Patch v1.1
Attachment #8365156 - Attachment is obsolete: true
(Assignee)

Comment 4

4 years ago
Comment on attachment 8365160 [details] [diff] [review]
Patch v1.1

Ok, I'm happier with this.
Attachment #8365160 - Flags: review?(gijskruitbosch+bugs)

Comment 5

4 years ago
Comment on attachment 8365160 [details] [diff] [review]
Patch v1.1

Review of attachment 8365160 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/osx/customizableui/panelUIOverlay.css
@@ +10,5 @@
>  
>  @media (min-resolution: 2dppx) {
> +  /* Wide items like the Cut/Copy/Paste and Zoom controls are special in that their icons
> +     are 16x16 when in the panel, but 18x18 when in a toolbar. */
> +  toolbaritem.panel-wide-item[cui-areatype="menu-panel"] > toolbarbutton > .toolbarbutton-icon {

This doesn't apply in the palette, and it *will* apply to add-on buttons. I think we should make this specific to our controls, and it should work in the palette as well as the panel.
Attachment #8365160 - Flags: review?(gijskruitbosch+bugs) → feedback+
(Assignee)

Comment 6

4 years ago
Ah yes, good call. Forgot about the palette.
(Assignee)

Comment 7

4 years ago
Created attachment 8365224 [details] [diff] [review]
Patch v1.2

Will do a self-review and then test on Linux and Windows...
Attachment #8365160 - Attachment is obsolete: true
(Assignee)

Comment 8

4 years ago
Created attachment 8365260 [details] [diff] [review]
Patch v1.3
Attachment #8365224 - Attachment is obsolete: true
(Assignee)

Comment 9

4 years ago
Comment on attachment 8365260 [details] [diff] [review]
Patch v1.3

Ok, this takes care of the palette case as well.

I had to modify customizeMode.inc.css because the margin was causing the icons to be squished. I tested this on Windows, Linux and OS X, and everything still looks fine (probably better, actually).
Attachment #8365260 - Flags: review?(gijskruitbosch+bugs)

Comment 10

4 years ago
Comment on attachment 8365260 [details] [diff] [review]
Patch v1.3

Review of attachment 8365260 [details] [diff] [review]:
-----------------------------------------------------------------

Excellent! I'd noticed the icon looking sad in customize mode the other day, but was too busy at the time to file it and forgot. Thanks!
Attachment #8365260 - Flags: review?(gijskruitbosch+bugs) → review+
(Assignee)

Comment 11

4 years ago
Thanks for the review! Landed on fx-team:

remote:   https://hg.mozilla.org/integration/fx-team/rev/05b43154936d
https://hg.mozilla.org/mozilla-central/rev/05b43154936d
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
You need to log in before you can comment on or make changes to this bug.