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

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

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.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1

This should unblur the icons on Retina, and save us a little bit of painting time to boot.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8365156 [details] [diff] [review]
Patch v1

On second thought, I think I like that rule in panelUIOverlay.inc.css instead.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1.1

Comment 4

[Mike Conley (:mconley) (:⚙️)]

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

Ok, I'm happier with this.

Comment 5

[:Gijs (he/him)]

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.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Ah yes, good call. Forgot about the palette.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1.2

Will do a self-review and then test on Linux and Windows...

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1.3

Comment 9

[Mike Conley (:mconley) (:⚙️)]

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).

Comment 10

[:Gijs (he/him)]

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!

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Thanks for the review! Landed on fx-team:

remote:   https://hg.mozilla.org/integration/fx-team/rev/05b43154936d

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/05b43154936d